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

instanceof Object fails? #1769

Open
jods4 opened this issue Mar 13, 2017 · 19 comments
Open

instanceof Object fails? #1769

jods4 opened this issue Mar 13, 2017 · 19 comments

Comments

@jods4
Copy link

jods4 commented Mar 13, 2017

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.

@Sebmaster
Copy link
Member

Sebmaster commented Mar 13, 2017

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.

@jods4
Copy link
Author

jods4 commented Mar 13, 2017

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

@Sebmaster
Copy link
Member

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

It's uuuh... very weird.

@jods4
Copy link
Author

jods4 commented Mar 13, 2017

In my debugger:

> obj.constructor === Object
false

@jods4
Copy link
Author

jods4 commented Mar 13, 2017

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?

@Sebmaster
Copy link
Member

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).

@jods4
Copy link
Author

jods4 commented Mar 13, 2017

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
Copy link
Author

jods4 commented Mar 14, 2017

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?

@Sebmaster
Copy link
Member

Sebmaster commented Mar 14, 2017

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.

@jods4
Copy link
Author

jods4 commented Mar 14, 2017

@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.

@Sebmaster
Copy link
Member

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.

@jods4
Copy link
Author

jods4 commented Mar 14, 2017

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?

@Sebmaster
Copy link
Member

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.

@jods4
Copy link
Author

jods4 commented Mar 14, 2017

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.

@Sebmaster
Copy link
Member

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).

@jods4
Copy link
Author

jods4 commented Mar 14, 2017

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.

@freeman
Copy link

freeman commented Dec 4, 2019

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

@jonnytest1
Copy link

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

@bentorkington
Copy link

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)
    })
})

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

5 participants