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

window is defined, breaking platform-detection in node environment. #71

Closed
johnjbarton opened this issue Dec 31, 2015 · 19 comments
Closed

Comments

@johnjbarton
Copy link

We have code that tests its environment:

if (typeof window === 'undefined') {
  // node stuff
} else {
  // browser stuff.
}

To use iron-node on node apps with such tests, window from iron-node cannot be visible to the app under debug.

@s-a
Copy link
Owner

s-a commented Dec 31, 2015

Check out famous libs like jquery or underscore. I suggest test the variable module

// Export the Underscore object for **CommonJS**, with backwards-compatibility
// for the old `require()` API. If we're not in CommonJS, add `_` to the
// global object.
if (typeof module !== 'undefined' && module.exports) {
        module.exports = _;
        root._ = _;
        isNode = true;
} else {
        root._ = _;
}

@s-a s-a closed this as completed Dec 31, 2015
@johnjbarton
Copy link
Author

Thanks. Unfortunately that kind of test will only work if the JS has been loaded with require(). (Node's require() function adds the module as a argument). Any node code that uses eval, eg REPLs, module loaders, transcoders, will fail.

It would be helpful if the documentation had a section on limitations, outlining that the node code runs in a browser environment.

@dhritzkiv
Copy link

I have a similar issue related to the presence of the window variable.

mongoose / mongodb do a check for window and determine whether to use the node's Buffer or a browser's UINT8Array. When window is present, mongoose stops working properly for me. Is there a way to disable the windows variable using the iron-node configuration options, or otherwise?

@s-a
Copy link
Owner

s-a commented Feb 4, 2016

@hendrikswan just reported the same issue.

This is a general concern that I've felt since coding ironNode. In fact we cannot overwrite window and this usecase is the exact opposite to #90 where window is usefull. So I don't see any easy fix... Please feel free to brainstorm!

@s-a s-a reopened this Feb 4, 2016
@hendrikswan
Copy link

@s-a is it even possible to do something about this? Does the global window var come from electron?

Update: Sorry was busy typing this question while you were commenting :)

@s-a
Copy link
Owner

s-a commented Feb 4, 2016

Yes this is the result of Electron ' s hybrid environment.

@johnjbarton
Copy link
Author

iron-node is providing both process orchestration -- one command debugs my nodejs app -- and attachment of Chrome Devtools to that nodejs app. Unfortunately the attachment is kinda cheating: rather than running the nodejs in a nodejs environment iron-node runs the app in a hybrid browser environment so electron's own Chrome Devtools is ready at hand. (Its a great strategy by the way!)

To run the nodejs app in a nodejs environment iron-node would need to start the app in another process and attach the chrome devtools in the electron UI to that process via ipc or rpc. Unfortunately this step is not simple.

One issue that is unclear to me is how the Electron UI is involved. Are there two BrowserWindows in iron-node, one for the nodejs app being debugged and one for the iron-node UI? Mixing the iron-node UI with the nodejs app makes solving the window problem much harder.

@s-a
Copy link
Owner

s-a commented Feb 5, 2016

If this one http://stackoverflow.com/questions/35211056/how-to-change-the-node-js-module-wrapper would be possible we could easy pass a window parameter as undefined. Pls upvote there to get more attention. 👍

@johnjbarton
Copy link
Author

I don't see how the module wrapper can help here. window isn't a parameter, it's a global reference. (And I'm unsure if Chrome Devtools will work against a JS environment w/o window)

@s-a
Copy link
Owner

s-a commented Feb 5, 2016

While we use require each code file runs in an own context scope and if we could change the wrapper code and add window as argument node.js will pass undefined. So this problem would be solved.

@johnjbarton
Copy link
Author

There are some hacky ways to manipulate nodejs Module that are widely (ab)used, for example:
https://github.com/google/traceur-compiler/blob/master/src/node/require.js

@jonaswindey
Copy link

Having the same issue with the mongodb (js-bson) driver that fails when global.window is defined.

Anyone an idea why setting global.window = undefined doesn't fix this?

Edit; exactly like @dhritzkiv says

@dhritzkiv
Copy link

@jonaswindey have you tried updating to the latest mongodb? Updating mongoose (which included an updated mongodb) fixed this issue for me.

@jonaswindey
Copy link

Yes I did, I even forked mongoose to use a patched version of js-bson (https://github.com/officert/js-bson.git) that disabled the check on global.window

I keep getting MongoError {name: "MongoError", message: "data.copy is not a function"}

@dhritzkiv Which npm version do you use? I'm using node v5.2.0 but still npm 2.x

@s-a
Copy link
Owner

s-a commented Feb 18, 2016

Stay tuned . I will try some things out this weekend.

@s-a s-a closed this as completed in 5195b8b Feb 18, 2016
@s-a
Copy link
Owner

s-a commented Feb 18, 2016

Hey guys. could not wait to try this out. 😄

There are new options available now at

"window", "document", "self", "global", "navigator",

image

I think this is a pretty cool solution.
Can someone please install from master branch and test out the new feature to confirm that this works as expected before i publish this to npm?

@s-a
Copy link
Owner

s-a commented Feb 18, 2016

My tests performed well. Just published https://github.com/s-a/iron-node/releases/tag/v2.2.15 to npm.

@hendrikswan
Copy link

So far so good for me too!

On Thu, 18 Feb 2016 at 14:08 Stephan Ahlf notifications@github.com wrote:

My tests performed well. Just published
https://github.com/s-a/iron-node/releases/tag/v2.2.15 to npm.


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

@johnjbarton
Copy link
Author

This is much better, thanks! But there is still a bit more to do ;-) I'll open a new issue about global.

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

No branches or pull requests

5 participants