fix(zone.js): fix wrongly bundle rxjs.#35983
Closed
JiaLiPassion wants to merge 1 commit intoangular:masterfrom
Closed
fix(zone.js): fix wrongly bundle rxjs.#35983JiaLiPassion wants to merge 1 commit intoangular:masterfrom
JiaLiPassion wants to merge 1 commit intoangular:masterfrom
Conversation
JiaLiPassion
added
the
target: patch
mhevery
approved these changes
Mar 10, 2020
Contributor
mhevery
left a comment
mhevery
left a comment
There was a problem hiding this comment.
LGTM, but please clean up the comment:
fix(zone.js): fix `rxjs-patch` bundle to refer to `rxjs` (rather than include) it.
Close #35878.
Before zone.js 0.10, the rollup config would refer to `rxjs` when bundling `zone-patch-rxjs.js`
From zone.js 0.10, we started to use bazel to build `zone-patch-rxjs.js` and the configuration was wrongly defined to include a copy of `rxjs` in the `zone-patch-rxjs.js`.
Please verify that my comment make sense and it is consistent with what you are actually changing. (As I am only guessing here)
mhevery
added
action: cleanup
JiaLiPassion
force-pushed
the
zone-rxjs-bundle
branch
from
f77cece to
bd629f0
Compare
JiaLiPassion
removed
the
action: cleanup
Contributor
Author
|
@mhevery, thank you and I updated the commit message. |
… than include) it. Close angular#35878. Before zone.js 0.10, the rollup config would refer to `rxjs` when bundling `zone-patch-rxjs.js` From zone.js 0.10, we started to use bazel to build `zone-patch-rxjs.js` and the configuration was wrongly defined to include a copy of `rxjs` in the `zone-patch-rxjs.js`.
JiaLiPassion
force-pushed
the
zone-rxjs-bundle
branch
from
bd629f0 to
304379d
Compare
AndrewKushnir
pushed a commit
that referenced
this pull request
Mar 11, 2020
… than include) it. (#35983) Close #35878. Before zone.js 0.10, the rollup config would refer to `rxjs` when bundling `zone-patch-rxjs.js` From zone.js 0.10, we started to use bazel to build `zone-patch-rxjs.js` and the configuration was wrongly defined to include a copy of `rxjs` in the `zone-patch-rxjs.js`. PR Close #35983
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Close #35878.
Before zone.js 0.10, the rollup config when bundling
zone-patch-rxjs.jsdefinesrxjslibrary intoexternsandglobalsconfigurations, so rollup will not bundlerxjssource code intozone-patch-rxjs.jsbundle.From zone.js 0.10, we use bazel to build zone.js and the
externalsandglobalsconfigurationsare not correct, so the final bundle of
zone-patch-rxjsis not correct. The source code ofrxjsis also bundled, https://unpkg.com/[email protected]/dist/zone-patch-rxjs.jsthis PR recover the old rollup configurations. So the
zone-patch-rxjs.jswill only include thesource code from
zone.js.PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?