Skip to content
This repository has been archived by the owner on Mar 13, 2018. It is now read-only.

Use of global in ChangeSummary IIFEs breaks loading in node-webkit #23

Closed
dfreedm opened this issue Aug 5, 2013 · 9 comments
Closed

Comments

@dfreedm
Copy link
Contributor

dfreedm commented Aug 5, 2013

Moved Polymer/polymer#228 to here.

node-webkit is a project that combines node.js and Chromium, with node.js preloaded into the javascript vm.

node.js uses the global global scope, and starting at Line 1174 in change_summary.js, the use of global will put PathObserver and other modules into the node.js scope and not the window scope.

@richardanaya
Copy link

Verified, I was able to fix this just by doing a search replace on "global" to something else

@rafaelw
Copy link
Contributor

rafaelw commented Nov 19, 2013

I don't know anything about node-webkit. I'm going to close this issue, but ask anyone who this is hurting to submit a PR to fix it.

@rafaelw rafaelw closed this as completed Nov 19, 2013
@wilkerlucio
Copy link

I'm having this issue here, it actually starts with not finding ArraySplice... any hint on how to fix that? I really wanna use polymer with node-webkit...

@rafaelw
Copy link
Contributor

rafaelw commented Dec 2, 2013

The issue here isn't really observe-js, it's all of Polymer. Node's top-level scope isn't the global scope, so the only way to fix this would be to have polymer change to use a node-friendly package system, or to have all uses of symbols check for the symbol in the top-level scope and also on 'global.'

@rafaelw
Copy link
Contributor

rafaelw commented Dec 2, 2013

@mattsmcnulty

@cadorn
Copy link

cadorn commented Dec 2, 2013

change to use a node-friendly package system, or to have all uses of symbols check for the symbol in the top-level scope and also on 'global.'

I would not single out NodeJS here. NodeJS is not the only server environment and I can see polymer being used on others.

Check for exports global to detect CommonJS environment as a start.

@wilkerlucio
Copy link

agreed with @cadorn

@wilkerlucio
Copy link

actually I found a fix here, I got the compiled polymer file, search and replace "global" with something else (I used fakeGlobal, but almost any name will do the same). but it's hacky and ugly...

@FunkMonkey
Copy link
Contributor

The problem is the last line of observe.js, which tries to differentiate between node.js and the browser:

})(typeof global !== 'undefined' && global ? global : this || window);

In a node-webkit html page, both global and window exist. Unfortunately observe is polyfilled on global though it is needed on window. But as @cadorn pointed out, exports does not exist in a node-webkit html page, neither does module. And this points to window and not to exports as in a node.js module.

There are various ways to check for node-webkit now, one of them could simply be:

})(typeof global !== 'undefined' && global && typeof module !== 'undefined' && module ? global : this || window);

@rafaelw: As I see it, this issue is not effecting all of polymer, but only the javascript-specific polyfills like Object.observe which are designed to work in node.js too. All browser polyfills don't work in node anyway... Thus WeakMap should be the only other that might need such a fix, but looking at the source, it is currently browser-only anyway (relying on window)

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

No branches or pull requests

6 participants