[JS] Remove the global document variable

[oveddan]

Having a global document variable without implementing all of its methods causes interop issues with other libraries that expect the global document to have those methods, if it exists.

This occurs, for example, when trying to require jquery, and it sees a global document and attempts to call createElement on it. Use case and explaination of issue can be seen here.

According to @vjeux

There is no good reason for react-native to have a global document variable with just a single key createElement: null. We need to remove document altogether, it was added as a workaround very early in the project and is not needed anymore.

Therefore it seems the best solution would be to not attach the react-native specific document to global.

[vjeux]

Can you send a pull request to remove the line that adds it? It's in InitializeJavascript js file

[oveddan]

@vjeux sure I will look into this.

[sophiebits]

(We'll probably want to set it to undefined.)

[oveddan]

@vjeux @spicyj I dug around for a while - we don't set GLOBAL or window.document anywhere, even in InitializeJavaScriptAppEngine.js
All that we do with global.document is set createElement to null

I tried running window.document = undefined; but it doesn't work. This is because window.document is a read method.

If you try in the chrome or safari console to run the code:

window.document = undefined;
window.document;

You will see that window.document still returns the document.

The better solution would not to override document.createElement to be null.

[sophiebits]

We really don't want a document though for React Native. Annoying that it's non-writable, non-configurable.

[ide]

@jaredly I heard you were thinking about a new executor. There are a handful of issues like this one that could be closed by moving away from Chrome =)

[oveddan]

@spicyj a bit more on this...http://stackoverflow.com/questions/30196216/why-window-or-document-cant-be-set-to-undefined-or-null

[sophiebits]

@oveddan I understand. Still annoying. :)

[vjeux]

Maybe we could run in a web worker instead of the main js thread when debugging in chrome. This way we won't have document.

Not sure how the debugging experience is though

[oveddan]

@vjeux After your comment - I just confirmed that:
When using chrome debugger, window.document is not undefined, and you get an error when trying to set it.
Same goes for safari debugger.
When not debugging, window.document is already undefined.

If we wanted to have it run in the debugger from a web worker, it would need to be done for safari too.

I feel like this would adversely effect the debugging experience. Also it would probably have issues should people want to use karma, or any other browser-based tester, to run tests.

[jaredly]

Can't we just change Environment.canUseDOM to make it configurable? e.g. there be some global flag to disable it even in the presence of document etc.
As I understand it, that's the only reason we're messing with document. Are there other reasons that we want it not to be defined in chrome/safari?

[sophiebits]

@jaredly It's not defined in the JSC executor, so we don't want it available when debugging either, for consistency. React isn't the only one who sniffs this. As one example, jQuery behaves differently depending on whether the window and document globals are available.

[jaredly]

Cool. I'd say let's try a webworker then -- debugging in chrome is pretty well supported (don't know about safari)

[oveddan]

@spicyj good point - for consistency it would be best for document not to exist. @vjeux If you're not working on it yet, I'll take a stab at making it run in the web worker and see what the debugging experience is like.

[oveddan]

@vjeux @spicyj Take a look at the PR and let me know what you think. Debugging doesn't seem to be adversely affected in chrome.

[oveddan]

@spicyj On second look - the React tab is now gone from chrome when running the code in a worker, and using the debugger. Any suggestion on how to get it back?

[jaredly]

Yeah... That's probably going to be difficult with the current devtools.
We're looking at rearchitecting things though, in which case this would be
easier
On Mon, Jun 15, 2015 at 2:10 PM Dan Oved [email protected] wrote:

@spicyj https://github.com/spicyj On second look - the React tab is now
gone from chrome when running the code in a worker, and using the debugger.
Any suggestion on how to get it back?

—
Reply to this email directly or view it on GitHub
#1473 (comment)
.

[browniefed]

Just as a side note if someone stumbles upon this issue. Debugging socket.io with chrome dev tools open is apparently impossible impossible at the moment. It'll throw up the document.createElement is undefined even though a successful connection can be made. It works perfectly fine and connects with chrome debugging off.

[ccheever]

seems like #1632 is in limbo. Is this still an issue? If so should that PR go through or is a different approach needed?