-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Track updates of globals that are exported as default #3550
Conversation
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via
or load it into the REPL: |
Codecov Report
@@ Coverage Diff @@
## master #3550 +/- ##
==========================================
- Coverage 96.29% 96.29% -0.01%
==========================================
Files 176 176
Lines 6044 6043 -1
Branches 1783 1781 -2
==========================================
- Hits 5820 5819 -1
Misses 112 112
Partials 112 112
Continue to review full report at Codecov.
|
df0bdb8
to
2d6865a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that in ES modules, global = value
is not a supported global assignment (strict mode!).
Also export default global
should very much be supported as a snapshot at the time of execution and not a live binding.
Also see previous bug - #2000. |
Ah I see, but I think I can rework the test case so that we update the global variable in a legal way. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolves #2000
I see, yes I misread that - I did think I had posted an issue for this snapshotting exactly, but apparently not. |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Resolves #3534
Description
When a global variable is assigned to a default export, Rollup does not track updates of the global variable even though it should, creating an unintended live-binding to the global variable where you would actually want a snapshot:
https://rollupjs.org/repl/?version=2.9.0&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmV4cG9ydCUyMCU3QmRlZmF1bHQlMjBhcyUyMG9yaWdpbmFsJTJDJTIwdXBkYXRlZCU3RCUyMGZyb20lMjAnLiUyRmxpYiclM0IlMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCUyQyU3QiUyMm5hbWUlMjIlM0ElMjJsaWIuanMlMjIlMkMlMjJjb2RlJTIyJTNBJTIyZXhwb3J0JTIwZGVmYXVsdCUyMG15R2xvYmFsJTNCJTVDbm15R2xvYmFsJTIwJTNEJTIwMyUzQiU1Q25leHBvcnQlMjBjb25zdCUyMHVwZGF0ZWQlMjAlM0QlMjBteUdsb2JhbCUzQiUyMiUyQyUyMmlzRW50cnklMjIlM0FmYWxzZSU3RCU1RCUyQyUyMm9wdGlvbnMlMjIlM0ElN0IlMjJmb3JtYXQlMjIlM0ElMjJjanMlMjIlMkMlMjJuYW1lJTIyJTNBJTIybXlCdW5kbGUlMjIlMkMlMjJhbWQlMjIlM0ElN0IlMjJpZCUyMiUzQSUyMiUyMiU3RCUyQyUyMmdsb2JhbHMlMjIlM0ElN0IlN0QlN0QlMkMlMjJleGFtcGxlJTIyJTNBbnVsbCU3RA==
or even creating invalid code for ESM as you cannot directly reexport a global variable binding from a module. This is fixed by treating all global variables as having been (potentially) reassigned, which is true semantically.
Even though this is not the original issue of #3534, that issue will be solved here as well.