-
Notifications
You must be signed in to change notification settings - Fork 27k
fix(zone.js): fesm2015 bundle should also be strict module. #40456
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
Conversation
90fb7c1 to
3da9a7c
Compare
alan-agius4
left a comment
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.
LGTM.
Although I think it's simpler to just add it as part of the banner.
| * @license Angular v${version} |
The fact that rollup is removing the use strict pragma makes sense since ES modules are strict by default.
However the problem here is that when used with Webpack this is causing an issue. Webpack will typically add the use-strict pragma to ES modules, however Webpack is unable to determine that zone.js is fact an ES module because it doesn't contain any expressions such as import or export statements.
alan-agius4
left a comment
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.
LGTM.
Although I think it's simpler to just add it as part of the banner.
| * @license Angular v${version} |
The fact that rollup is removing the use strict pragma makes sense since ES modules are strict by default.
However the problem here is that when used with Webpack this is causing an issue. Webpack will typically add the use-strict pragma to ES modules, however Webpack is unable to determine that zone.js is fact an ES module because it doesn't contain any expressions such as import or export statements.
gkalpak
left a comment
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.
LGTM for dev-infra.
I can't really speak about the implementation, but @alan-agius4's suggestion of putting it in the banner sounds reasonable (and less "magic") to me.
Reviewed-for: dev-infra
3da9a7c to
31ddc25
Compare
|
@alan-agius4, @gkalpak , thanks for the review. I have updated the PR. such as Which should not be a real issue. |
ff09427 to
0a775c8
Compare
Close angular#40215 `fesm2015/zone.js` is built to `esm` bundle with rollup, so the 'use strict'; statement is not generated in the bundle, even we put the 'use strict' in the src code, rollup removes the code in the final bundle. So if we load the `fesm2015/zone.js` as a module, such as `ng serve`, in the index.html ``` <script src="polyfills.js" type="module"></script> ``` Everything works fine, since polyfills.js is loaded as `module`, so it is always `strict`. But in `ng test`, webpack concat the `zone.js` and loaded into the karma html. For other app and test code, they are still `strict` since they are `module` because they have `export/import` statement, but `zone.js` is a bundle without `export`, it is a `side effect` bundle, so after loaded by webpack, it becomes non-strict. Which causes some issues, such as angular#40215, the root cause is the `this` context should be `undefined` but treated as `Window` in `non-strict` mode. ``` Object.prototype.toString.apply(undefined); // should be [object undefined], but it is [object Window] in non-strict mode. // zone.js patched version of toString Object.prototype.toString = function() { ... // in non-strict mode, this is Window return originalObjectPrototypeToString.call(this); } ``` So in this commit, `'use strict';` is always added to the `esm` bundles.
0a775c8 to
95cb560
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Close #40215
fesm2015/zone.jsis built toesmbundle with rollup, so the 'use strict';statement is not generated in the bundle, even we put the 'use strict' in the src code,
rollup removes the code in the final bundle.
So if we load the
fesm2015/zone.jsas a module, such asng serve, in the index.htmlEverything works fine, since polyfills.js is loaded as
module, so it is alwaysstrict.But in
ng test, webpack concat thezone.jsand loaded into the karma html. For other app andtest code, they are still
strictsince they aremodulebecause they haveexport/importstatement, but
zone.jsis a bundle withoutexport, it is aside effectbundle, so afterloaded by webpack, it becomes non-strict. Which causes some issues, such as #40215,
the root cause is the
thiscontext should beundefinedbut treated asWindowinnon-strictmode.So this commit always add
'use strict';statement at the beginning of the zone.js es2015 bundle.