Use universal method of finding the global object #2132

Merged
merged 4 commits into from Sep 15, 2016

Projects

None yet

3 participants

@cyco
Contributor
cyco commented Aug 25, 2016 edited

The module store, runtime and symbol polyfills need access to the global object. This was done by checking the existance of window, global, self and falling back to this if none were defined. This can fail for two reasons. First, this should be expected to be set to undefined during module definitions (as is the case for symbols.js), thus rendering the fallback useless. Secondly some JavaScript implementations (e.g. JavaScript-Core Framework) might not even define a name for the global object that could be accessed within modules.
By using the Function-Constructor we create a new function in non-strict mode which has its thisArg set to the gobal object if no other thisArg is provided.

@googlebot
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.
@googlebot googlebot added the cla: no label Aug 25, 2016
@cyco
Contributor
cyco commented Aug 25, 2016

I signed it!

@googlebot
Collaborator

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Aug 25, 2016
@arv
Collaborator
arv commented Sep 3, 2016

Thank you for the patch.

One issue with this approach is that it does not work when eval is disabled (which is the case of CSP). So it needs to be wrapped in a try/catch with a fallback to the current ugly behavior.

@cyco
Contributor
cyco commented Sep 5, 2016

Thanks for your feedback. I'll add the change you suggested and update the PR.

@arv
Collaborator
arv commented Sep 14, 2016

I like it 👍

Can you also add an entry to AUTHORS?

@cyco
Contributor
cyco commented Sep 14, 2016

Done :)

@arv
Collaborator
arv commented Sep 15, 2016

Can you fix the merge conflict? Thanks.

@arv arv self-assigned this Sep 15, 2016
cyco added some commits Aug 25, 2016
@cyco cyco Use universal method of finding the global object
The module store, runtime and symbol polyfills need access to the global
object. This was done by checking the existance of window, global, self
and falling back to this if none were defined. This can fail for two
reasons. First, 'this' should be expected to be set to undefined during
module definitions (as is the case for symbols.js), thus rendering the
fallback useless. Secondly some JavaScript implementations (e.g. JavaScript
-Core Framework) might not even define a name for the global object that
could be accessed within modules.
By using the Function-Constructor we create a new function in non-strict
mode which has its thisArg set to the gobal object if no other thisArg
is provided.
74b88c6
@cyco cyco Improve search for global object.
Adds a try-catch block around new Function-call to catch any
SecurityError exceptions that are thrown if CSP is used.
f617616
@cyco cyco Add fallback for global detection in runtime
21058fe
@cyco cyco Add myself to AUTHORS
24388ab
@cyco
Contributor
cyco commented Sep 15, 2016

The conflict should be resolved now.

@arv
arv approved these changes Sep 15, 2016 View changes

Thanks

@arv arv merged commit caa7b75 into google:master Sep 15, 2016

2 checks passed

cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment