Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOMParser fails when used in a promise in Node.js #222

Open
luisespinal opened this issue Sep 20, 2017 · 1 comment
Open

DOMParser fails when used in a promise in Node.js #222

luisespinal opened this issue Sep 20, 2017 · 1 comment

Comments

@luisespinal
Copy link

DOMParser fails to bind to the proper "this" object when a DOMParser object is created outside of a promise, but used within a "thenable" block.

For instance.

const DOMParser = require('xmldom').DOMParser;
const parser= new DOMParser();
Promise.resolve(true)
                .then(readSomeFile)
                .then(parser.parseFromString)

Will cause the following error to occur.

(node:22420) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot read property 'domBuilder' of undefined
(node:22420) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

And that is because the "this" object has changed from where the parser was created and from the moment the promise is being evaluated. The error is on the third line below (in dom-parser.js)

DOMParser.prototype.parseFromString = function(source,mimeType){
	var options = this.options;
	var sax =  new XMLReader();
	var domBuilder = options.domBuilder || new DOMHandler();//contentHandler and LexicalHandler

The options variable is going to be undefined (because the "this" object referring to the context being executed in the current "thenable" block does not have such a property. Then when we try to create domBuilder, it inevitably breaks.

  1. One solution is to lazy evaluate construction of the parser till the last moment (not always possible)
const DOMParser = require('xmldom').DOMParser;
Promise.resolve(true)
                .then(readSomeFile)
                .then((x)=>{return (new DOMParser()).parseFromString(x)}) // meh, ok sometimes
  1. Or, if you must instantiate the parser outside of the thenable blocks, to lazily and explicitly bind to
    it
const DOMParser = require('xmldom').DOMParser;
const parser= new DOMParser();
Promise.resolve(true)
                .then(readSomeFile)
                .then(function(){return DOMParser.parseFromString.apply(parser,arguments);}) // UGLY!!!!
  1. Better yet, to bind parseFromString at the moment of construction in dom-parser.js, changing
    the constructor from this:
function DOMParser(options){
	this.options = options ||{locator:{}};
}

To this:

function DOMParser(options){
	this.options = options ||{locator:{}};
	var me = this;
	this.parseFromString = function(){ 
		return DOMParser.prototype.parseFromString.apply(me,arguments); 
	};
}

I've tested all changes, and my preference would be the last one.

Until (and if) such a change is incorporated, the workaround I'm using is number one and two above, depending on context.

luisespinal pushed a commit to luisespinal/xmldom that referenced this issue Sep 20, 2017
…e helps DOMParser bind to the proper "this" object even when executed within a promise or callback.
luisespinal pushed a commit to luisespinal/xmldom that referenced this issue Sep 20, 2017
…e helps DOMParser bind to the proper "this" object even when executed within a promise or callback.
@markstos
Copy link

markstos commented Oct 9, 2017

I would expect the behavior you experience. It's common to need to explicitly bind a method to object when passing the method as a function. Rather than patching this code, I would expect a bound function to passed into the promise:

    parser.parseFromString.bind(parser)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants