Closed dead-claudia closed 8 years ago
Like all modern JS, the polyfill is intended to be run via a module system, so creating global variables shouldn't be a concern - I'm not convinced that supporting a script context is important.
Also, with the use of Function
, this code isn't CSP-safe at all, so I'm not sure what you're referring to there.
@ljharb
First, I think supporting a script context would make it relatively easy to simply throw into a browser. Instead of it being purely for Node, Browserify, etc., it could be used as a simple drop-in wherever you're running JS. It could be dropped by itself into a script tag, for example, and that's less boilerplate in the entry script.
Second, I believe that the Function
constructor doesn't run unless the top-level this
is not an object (i.e. it's being run in strict mode). The only way this would get caught by CSP is through static analysis, which most common browsers don't do, or else the common eval
test wouldn't work. Some things like Caja do that, though, but they often go up and beyond the CSP world.
The only way the Function
constructor gets called is if this becomes part of the body of a strict mode function. The only way I can think of to do that is via Browserify and making the global execution context in strict mode before this gets run. Otherwise, it's CSP-proof.
Convincing points.
The added closure keeps from introducing
global
as a global variable in browsers and shells. Which makes this more portable.Also, testing that
this
is an object works better when loaded from a script context, as creating a function is unnecessary unless in a module context. Because of that, this is CSP safe unless the global scope is in strict mode already (which is bad practice from the start).