jsdom icon indicating copy to clipboard operation
jsdom copied to clipboard

instanceof Object fails?

Open jods4 opened this issue 8 years ago • 18 comments

Running a webapp inside JSDOM, I observe the following test failing:

let input: HTMLInputElement = ...; // got this from the DOM
let ok = input instanceof Object; // true in a browser, false in JSDOM

Does it make any sense?

I observe that the prototype chain of the "fake" HTMLInputElement goes to a fake Object and not the real Node one, but shouldn't the Object in the code above also point to the same fake object?

To be clear: this code is running inside the webapp+JSDOM, not in the main Node code.

jods4 avatar Mar 13 '17 21:03 jods4

Sadly, prototype chains are screwed up due to sandboxing, which we can't really fix (without making jsdom a whole lot slower).

The Object in the HTMLInputElement chain is the "real" node Object outside of the sandbox. The Object you're referencing directly is the Object inside the sandbox.

Sebmaster avatar Mar 13 '17 22:03 Sebmaster

HTMLInputElement chain is the "real" node Object outside of the sandbox

Is it really? that's not what I see from a debugger:

> input
  HTMLInputElement {undefined: HTMLInputElementImpl}

> var obj = input.__proto__.__proto__.__proto__.__proto__.__proto__.__proto__;

> obj
Object {__defineGetter__: function __defineGetter__() { [native code] }, ... }

> obj === Object.prototype
false

Edited because I'm stupid

jods4 avatar Mar 13 '17 22:03 jods4

> obj = input.__proto__.__proto__.__proto__.__proto__.__proto__.__proto__
{}
> obj instanceof Object
false
> obj.constructor == Object
true

It's uuuh... very weird.

Sebmaster avatar Mar 13 '17 22:03 Sebmaster

In my debugger:

> obj.constructor === Object
false

jods4 avatar Mar 13 '17 22:03 jods4

Everything looks consistent in my debugger. Simply the end of the proto chain of HTMLInputElement is not Node's global Object. In itself it's not bad, but apparently it's not the Object scripts use when doing x instanceof Object and that's bad :( At least that's my understanding of this, but I'm not well versed into JSDOM internals so I might be wrong somewhere. I was suspicious that the Acorn code that modifies free global variables into window.XXX did not work for instanceof X?

jods4 avatar Mar 13 '17 22:03 jods4

Yeah, we've had this problem with a few other things, namely /abc/ instanceof Regexp and similiar things. Sadly the only thing we (or you, in the created callback) can do here is patch node's global Object into the jsdom environment (like window.Object = Object).

Sebmaster avatar Mar 13 '17 22:03 Sebmaster

Hmmm.... maybe that's the problem actually, not the solution! Strangely:

>Object === window.Object
true

So window.Object is the Node Object indeed, but somehow HtmlInputElement is not rooted in the Node Object but in another Object?? Why isn't that other Object the window.Object?

jods4 avatar Mar 13 '17 22:03 jods4

I am very confused now, but if I set window.Object = Object in the created event. Can someone explain in more details what actually happens? Is this an acceptable fix? If so, why is it not inside JSDOM?

jods4 avatar Mar 14 '17 00:03 jods4

If you do that then !({} instanceof Object) which is really bad usually in JS. There's no real general solution here at the moment, it's all a tradeoff.

Sebmaster avatar Mar 14 '17 00:03 Sebmaster

@Sebmaster I can't make sense of what I have observed in the debugger but anyway...

You said the prototype chain of HTMLInputElement does go the global Object rather than window.Object, why not? It would not be hard to set it up that way, what's the problem?

What does JSDOM do to make !({} instanceof Object)? It's certainly no a good behavior, indeed.

jods4 avatar Mar 14 '17 00:03 jods4

It would not be hard to set it up that way, what's the problem?

It would require setting up a whole new prototype chain for every jsdom window. require('jsdom') is already a very slow call, just imagine that for every new window instance (not quite, cause you can save parsing and fs costs, but roundabout). It'd be nice to have that configurable, but we just don't have the code structure for that (yet).

That latter point would happen if you overwrite window.Object.

Sebmaster avatar Mar 14 '17 00:03 Sebmaster

It would require setting up a whole new prototype chain for every jsdom window

Not necessarily. Instead of sharing the global Object you could share a singleton window.Object. That way:

  • the prototype chain is defined only once
  • instanceof works
  • you have isolation from global but shared windows.

That latter point would happen if you overwrite window.Object.

I got that but why? What hackery enables you to change the prototype of a {} literal?

jods4 avatar Mar 14 '17 00:03 jods4

Instead of sharing the global Object you could share a singleton window.Object.

Yeah, that would also work. We thought about this a bit. By extending it to also overwrite Function prototypes and similar you could even get the sandbox somewhat safe which would be nice. No one's done the work though.

I got that but why? What hackery enables you to change the prototype of a {} literal?

It's not changing the prototype of {} (that's defined by the sandbox), it's changing what is referenced by Object.

Sebmaster avatar Mar 14 '17 00:03 Sebmaster

just thinking... or do you want non-shared window.Object? You can't have everything I guess. If you want to isolate window.Object and re-use HTMLInputElement & co for perf reason that's obviously conflicting. Indeed an option for correctness over startup time would be nice.

Another idea: we could mitigate that problem by using Symbol.hasInstance. With this we could hack the window.Object so that instanceof actually returns true when queried with an instance of the global Object.

jods4 avatar Mar 14 '17 01:03 jods4

hasInstance is a kinda shitty patch though, it just hides the problem a bit better. Walking the prototype chain would still not work for example.

or do you want non-shared window.Object?

That'd be the optimal solution. But we already have issues with Startup time, both in the require and in the actual document creation, so making that worse is not really an option, which leads to the though that this has to be a config option. Either for all 3 modes, or just 2 of them (shared node-object + separate objects per window).

Sebmaster avatar Mar 14 '17 01:03 Sebmaster

Walking the prototype chain would still not work for example.

Agreed, but it would still be better than today. At least instanceof would work. Whereas today none of those two work. That's already an improvement and I'd argue instanceof is easier and hence more common than walking the prototype chain.

jods4 avatar Mar 14 '17 01:03 jods4

Something that works for me is this in jest.config.js:

module.exports = {
  // ...
  globals: {
    Object: Object,
  },
  // ...
}

thanks to some code in jest-environment-jsdom-fifteen

freeman avatar Dec 04 '19 15:12 freeman

is this the same issue ?

global.Text = new JSDOM('test', { contentType: 'text/html' }).window.Text;
const doc=new JSDOM("test", { contentType: "text/html"}).window.document

doc.childNodes[0] instanceof Text // => false

// doc.childNodes[0].constructor = TextImpl(Symbol) - which is apparently not a subtype

jonnytest1 avatar Jan 21 '22 12:01 jonnytest1

This help?

copy the HTML*Element (etc.) types from the jsdom object and inject them into the global namespace before instanceof gets called. e.g. with mocha:

const { JSDOM } = require('jsdom')

const { instanceofCheck } = require('./instanceofCheck')
// const instanceofCheck = x => x instanceof window.HTMLElement

describe('JSDOM tests', () => {
    let jsDomInstance

    beforeEach(() => {
        jsDomInstance = new JSDOM()
        global.HTMLElement = jsDomInstance.window.HTMLElement
    })

    it('passes instanceof check', () => {
        expect(
            instanceofCheck(
                jsDomInstance.window.document.createElement('div')
            )
        ).toBe(true)
    })
})

bentorkington avatar Oct 17 '22 00:10 bentorkington