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

Don't use window object #9

Closed
wants to merge 2 commits into from
Closed

Don't use window object #9

wants to merge 2 commits into from

Conversation

kkaefer
Copy link

@kkaefer kkaefer commented Oct 23, 2013

The window object doesn't exist in all JavaScript contexts, e.g. Web workers.

The window object doesn't exist in all JavaScript contexts, e.g. Web workers.
@mourner
Copy link
Owner

mourner commented Oct 23, 2013

self is not a part of any specification, so in theory it can break in some browsers... Maybe lets use this?

@kkaefer
Copy link
Author

kkaefer commented Oct 23, 2013

this is not a globally defined variable.

@mourner
Copy link
Owner

mourner commented Oct 23, 2013

But this should work in the top context, so many people do something like this:

(function (window) {
})(this);

I remember there were some issues with this approach too (when including Leaflet in browserify or something), @tmcw, comments?

@tmcw
Copy link

tmcw commented Oct 23, 2013

I usually rely on umd template to do the right thing and it does.

@patrickarlt
Copy link

We are using an adapted UMD template in Terraformer to make sure thing work in both node and browser environment.

This seems to work pretty well, here is the code...

(function (root, factory) {

  // Node.
  if(typeof module === 'object' && typeof module.exports === 'object') {
    exports = module.exports = factory();
  }

  // Browser Global.
  if(typeof window === "object") {
    root.Terraformer = factory();
  }

}(this, function(){

  //export stuff

  return exports;
}));

@mourner
Copy link
Owner

mourner commented Oct 24, 2013

OK, found the Leaflet commit by @jfirebaugh Leaflet/Leaflet@e699894
It seems that in this case it's OK to pass this, as there are no window references except for exporting, which makes it compatible with browserify even without shimming. Right?

@jfirebaugh
Copy link

Yes, IIRC the leaflet change was necessary because it wanted to access actual window properties, not just attach exported properties to it.

@kkaefer
Copy link
Author

kkaefer commented Oct 25, 2013

this is not defined in a web worker global context.

@mourner mourner closed this in 615af09 Oct 25, 2013
@mourner
Copy link
Owner

mourner commented Oct 25, 2013

Wow, I thought it was defined... Thanks Konstantin. Added export through self.
Not sure why there's a "global" export in the UMD template mentioned by @tmcw, seems not necessary.

@mourner
Copy link
Owner

mourner commented Oct 28, 2013

BTW, regarding the export through global — the umd guys admitted that it's useless. ForbesLindesay/umd@4464d83

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

Successfully merging this pull request may close these issues.

5 participants