Hotfix: Do not deconflict "undefined" #2291
Conversation
src/ast/scopes/Scope.ts
Outdated
| this?: ThisVariable | LocalVariable; | ||
| default?: ExportDefaultVariable; | ||
| arguments?: ArgumentsVariable; | ||
| undefined?: UndefinedVariable; |
There was a problem hiding this comment.
I left this out previously, because undefined can actually be redefined as a variable identifier, so in theory we should allow it to be deshadowed.
There was a problem hiding this comment.
Ah yes, removed the type from the list. I think redeclarations should be handled properly, though, as a local declaration would of course shadow the global one now.
| } | ||
|
|
||
| getLiteralValueAtPath(): LiteralValueOrUnknown { | ||
| return undefined; |
There was a problem hiding this comment.
What cases would this apply to?
There was a problem hiding this comment.
This applies to all situations where the global undefined variable is used.
There was a problem hiding this comment.
That is all situations where undefined is accessed and has not been redeclared.
lukastaegert
force-pushed
the
do-not-deconflict-undefined
branch
from
824332b
to
c498dd2
Compare
|
Added a simple test where "undefined" is used as a function parameter and set to "true" via the function call to make sure we do not break this important functionality in the future 😜 |
|
Glad to know we're covering that important treeshaking scenario for our users! |
This Pull Request updates dependency [rollup](https://github.com/rollup/rollup) from `v0.60.7` to `v0.61.1` <details> <summary>Release Notes</summary> ### [`v0.61.1`](https://github.com/rollup/rollup/blob/master/CHANGELOG.md#​0611) [Compare Source](rollup/rollup@v0.61.0...697f36d) *2018-06-21* * Do not try to deconflict "undefined" ([#​2291](`rollup/rollup#2291)) * Properly track values for loop interator declarations and reassigned namespaces, add smoke test ([#​2292](`rollup/rollup#2292)) --- ### [`v0.61.0`](https://github.com/rollup/rollup/blob/master/CHANGELOG.md#​0610) [Compare Source](rollup/rollup@v0.60.7...v0.61.0) *2018-06-20* * Declare file dependencies via transform plugin hooks ([#​2259](`rollup/rollup#2259)) * Handle undefined values when evaluating conditionals ([#​2264](`rollup/rollup#2264)) * Handle known undefined properties when evaluating conditionals ([#​2265](`rollup/rollup#2265)) * Access watch events via the plugin context ([#​2261](`rollup/rollup#2261)) * Add option to suppress `__esModule` flag in output ([#​2287](`rollup/rollup#2287)) * Fix issue when re-declaring variables, track reassignments in more cases ([#​2279](`rollup/rollup#2279)) * Add VSCode debug settings ([#​2276](`rollup/rollup#2276)) --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com).
Resolves #2290
With the new "undefined" literal logic, undefined was tracked as a variable on module level which lead to instances in different modules being deconflicted. Very simple reproduction:
https://rollupjs.org/repl?version=0.61.0&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMCU3QmZvbyU3RCUyMGZyb20lMjAnLiUyRmZvby5qcyclM0IlNUNuJTVDbmNvbnNvbGUubG9nKGZvbyUyQyUyMHVuZGVmaW5lZCklM0IlMjIlN0QlMkMlN0IlMjJuYW1lJTIyJTNBJTIyZm9vLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmV4cG9ydCUyMGNvbnN0JTIwZm9vJTIwJTNEJTIwdW5kZWZpbmVkJTNCJTIyJTdEJTVEJTJDJTIyb3B0aW9ucyUyMiUzQSU3QiUyMmZvcm1hdCUyMiUzQSUyMmVzJTIyJTJDJTIybmFtZSUyMiUzQSUyMm15QnVuZGxlJTIyJTJDJTIyZ2xvYmFscyUyMiUzQSU3QiU3RCUyQyUyMmFtZCUyMiUzQSU3QiUyMmlkJTIyJTNBJTIyJTIyJTdEJTdEJTJDJTIyZXhhbXBsZSUyMiUzQW51bGwlN0Q=
This fixes this by declaring it as a global variable instead.