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

Avoid using eval() to get global object. #65

Merged
merged 1 commit into from
Sep 17, 2019
Merged

Avoid using eval() to get global object. #65

merged 1 commit into from
Sep 17, 2019

Conversation

louh
Copy link
Contributor

@louh louh commented Aug 28, 2019

Following up from #64, this PR avoids using eval() to get the global object, which causes this package to be unusable if a server has set Content-Security Policy (CSP) protocols. I use the Universal Module Definition (UMD) pattern, bringing back the way we inject the global object into the function via its arguments.

I should also note that there is a stage-3 ECMAScript proposal for globalThis, which is meant to address exactly this problem. Some browsers and environments have begun to implement globalThis but even if we were to use it now, we would still need a fallback to self or this. Let me know if you would like to use globalThis now, but I can also understand waiting for the proposal to be finalized first.

Let me know if there's anything else I should do to finish this PR! Thanks!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 94.444% when pulling 48437ba on louh:louh/global into d5ebbe4 on davidbau:released.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 94.444% when pulling 48437ba on louh:louh/global into d5ebbe4 on davidbau:released.

@DiskImage
Copy link

@davidbau any plans to merge this in soon? I'm also on a project that needs this eval removed for CSP reasons and it would be great to not have to maintain a fork in parallel to this!

@davidbau davidbau changed the base branch from released to master September 17, 2019 10:21
@davidbau davidbau merged commit 5f2ec42 into davidbau:master Sep 17, 2019
@davidbau
Copy link
Owner

Look great. Thanks for the PR!

@yuntonyx
Copy link

Amazing, thank you @louh and @davidbau !

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