refactor(upgrade): rename module constant to avoid webpack bug
#30107
Conversation
This file contains 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
gkalpak
added
area: upgrade
|
merge-assistance: The changes have been reviewed/approved as part of the original PR (#30058). |
gkalpak
force-pushed
the
7.2.x-refactor-upgrade-rename-module
branch
from
f39066d
to
b915212
Compare
gkalpak
added
state: WIP
and removed
action: merge
|
You can preview f39066d at https://pr30107-f39066d.ngbuilds.io/. |
|
You can preview b915212 at https://pr30107-b915212.ngbuilds.io/. |
…ular#30058) When targeting ES2015 (as is the default in cli@8), `const` is not downleveled to `var` and thus declaring `const module` throws an error due to webpack wrapping the code in a function call with a `module` argument (even when compiling for the `web` environment). Related: webpack/webpack#7369 Fixes angular#30050 PR Close angular#30058
gkalpak
force-pushed
the
7.2.x-refactor-upgrade-rename-module
branch
from
b915212
to
6bab93e
Compare
gkalpak
added
action: merge
|
You can preview 6bab93e at https://pr30107-6bab93e.ngbuilds.io/. |
…st` is in progress (angular#29794) Previously, under certain circumstances, `NgZone#onMicrotaskEmpty` could emit while a `$digest` was in progress, thus triggering another `$digest`, which in turn would throw a `$digest already in progress` error. Furthermore, throwing an error from inside the `onMicrotaskEmpty` subscription would result in unsubscribing and stop triggering further `$digest`s, when `onMicrotaskEmpty` emitted. Usually, emitting while a `$digest` was already in progress was a result of unintentionally running some part of AngularJS outside the Angular zone, but there are valid (if rare) usecases where this can happen (see angular#24680 for details). This commit addresses the issue as follows: - If a `$digest` is in progress when `onMicrotaskEmpty` emits, do not trigger another `$digest` (to avoid the error). `$evalAsync()` is used instead, to ensure that the bindings are evaluated at least once more. - Since there is still a high probability that the situation is a result of programming error (i.e. some AngularJS part running outside the Angular Zone), a warning will be logged, but only if the app is in [dev mode][1]. [1]: https://github.com/angular/angular/blob/78146c189/packages/core/src/util/ng_dev_mode.ts#L12 Fixes angular#24680 PR Close angular#29794
With angular#30058, the ngUpgrade internal `angular.module()` method wa renamed to `angular.module_()` (to avoid a webpack bug). Merging angular#29794 afterwards resulted in some broken tests, because it still used the old `angular.module()` method name. (The PR had been tested on CI against a revision that did not contain the rename.) This commit fixes the broken tests by renaming the remaining occurrences of `angular.module()`.
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the ℹ️ Googlers: Go here for more info. |
This was referenced Apr 25, 2019
|
You can preview f7b9406 at https://pr30107-f7b9406.ngbuilds.io/. |
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 25, 2019
) (#30107) When targeting ES2015 (as is the default in cli@8), `const` is not downleveled to `var` and thus declaring `const module` throws an error due to webpack wrapping the code in a function call with a `module` argument (even when compiling for the `web` environment). Related: webpack/webpack#7369 Fixes #30050 PR Close #30058 PR Close #30107
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 25, 2019
…st` is in progress (#29794) (#30107) Previously, under certain circumstances, `NgZone#onMicrotaskEmpty` could emit while a `$digest` was in progress, thus triggering another `$digest`, which in turn would throw a `$digest already in progress` error. Furthermore, throwing an error from inside the `onMicrotaskEmpty` subscription would result in unsubscribing and stop triggering further `$digest`s, when `onMicrotaskEmpty` emitted. Usually, emitting while a `$digest` was already in progress was a result of unintentionally running some part of AngularJS outside the Angular zone, but there are valid (if rare) usecases where this can happen (see #24680 for details). This commit addresses the issue as follows: - If a `$digest` is in progress when `onMicrotaskEmpty` emits, do not trigger another `$digest` (to avoid the error). `$evalAsync()` is used instead, to ensure that the bindings are evaluated at least once more. - Since there is still a high probability that the situation is a result of programming error (i.e. some AngularJS part running outside the Angular Zone), a warning will be logged, but only if the app is in [dev mode][1]. [1]: https://github.com/angular/angular/blob/78146c189/packages/core/src/util/ng_dev_mode.ts#L12 Fixes #24680 PR Close #29794 PR Close #30107
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 25, 2019
With #30058, the ngUpgrade internal `angular.module()` method wa renamed to `angular.module_()` (to avoid a webpack bug). Merging #29794 afterwards resulted in some broken tests, because it still used the old `angular.module()` method name. (The PR had been tested on CI against a revision that did not contain the rename.) This commit fixes the broken tests by renaming the remaining occurrences of `angular.module()`. PR Close #30107
|
This PR is merged to patch branch. |
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Backport of #30058 to 7.2.x.
UPDATE: This now, also, includes the changes from #29794 and #30126.