fix(upgrade): Do not trigger duplicate navigation events from Angular… #43441
+242
−49
Conversation
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
atscott
added
the
area: upgrade
atscott
force-pushed
the
duplicateUpgradeNav
branch
6 times, most recently
from
81c016e to
ace58dd
Compare
gkalpak
reviewed
Oct 16, 2021
| new $locationShimProvider(ngUpgrade, location, platformLocation, urlCodec, locationStrategy); | ||
|
|
||
| $locationProvider.hashPrefix(config && config.hashPrefix); | ||
| $locationProvider.html5Mode(config && !config.useHash); |
There was a problem hiding this comment.
Is it intentional that HTML5 mode is turned off if config is not passed in?
There was a problem hiding this comment.
I don't actually know. This was a straight copy-paste job from the upgrade_location_test_module file in the common package.
atscott
force-pushed
the
duplicateUpgradeNav
branch
from
73828d7 to
ae63fe1
Compare
|
@gkalpak Could you take another look at this? |
gkalpak
approved these changes
Feb 1, 2022
atscott
force-pushed
the
duplicateUpgradeNav
branch
from
c18a837 to
3f8a51b
Compare
This updates the router upgrade tests to use less mocked behavior. The test upgrade location module is copied from the one that's used in the common package. This update to the tests verifies more real behavior of the upgrade module.
… Router This code mimics behavior that Google Analytics has been using to prevent duplicate navigations. They set up their own `HybridRoutingService` location sync to avoid duplicate navigations that came from the Angular router. This would happen because the Angular router would trigger a navigation, which would then get picked up by the `$locationShim`, which would trigger a `$locationChangeStart`, which would then be picked up by the `setUpLocationSync` watcher here, which would again trigger a navigation in the Angular Router. All of this can be prevented by checking if the `navigationId` exists on the history state object. This property is added by the Angular router during navigations. fixes angular#21610
atscott
force-pushed
the
duplicateUpgradeNav
branch
from
3f8a51b to
8ec981c
Compare
|
This PR was merged into the repository by commit 202a1a5. |
jessicajaniuk
pushed a commit
that referenced
this pull request
Feb 2, 2022
This updates the router upgrade tests to use less mocked behavior. The test upgrade location module is copied from the one that's used in the common package. This update to the tests verifies more real behavior of the upgrade module. PR Close #43441
jessicajaniuk
pushed a commit
that referenced
this pull request
Feb 2, 2022
… Router (#43441) This code mimics behavior that Google Analytics has been using to prevent duplicate navigations. They set up their own `HybridRoutingService` location sync to avoid duplicate navigations that came from the Angular router. This would happen because the Angular router would trigger a navigation, which would then get picked up by the `$locationShim`, which would trigger a `$locationChangeStart`, which would then be picked up by the `setUpLocationSync` watcher here, which would again trigger a navigation in the Angular Router. All of this can be prevented by checking if the `navigationId` exists on the history state object. This property is added by the Angular router during navigations. fixes #21610 PR Close #43441
jessicajaniuk
pushed a commit
that referenced
this pull request
Feb 2, 2022
… Router (#43441) This code mimics behavior that Google Analytics has been using to prevent duplicate navigations. They set up their own `HybridRoutingService` location sync to avoid duplicate navigations that came from the Angular router. This would happen because the Angular router would trigger a navigation, which would then get picked up by the `$locationShim`, which would trigger a `$locationChangeStart`, which would then be picked up by the `setUpLocationSync` watcher here, which would again trigger a navigation in the Angular Router. All of this can be prevented by checking if the `navigationId` exists on the history state object. This property is added by the Angular router during navigations. fixes #21610 PR Close #43441
crapStone
pushed a commit
to Calciumdibromid/CaBr2
that referenced
this pull request
Feb 15, 2022
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`13.2.1` -> `13.2.2`](https://renovatebot.com/diffs/npm/@angular%2fanimations/13.2.1/13.2.2) | | [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`13.2.1` -> `13.2.2`](https://renovatebot.com/diffs/npm/@angular%2fcommon/13.2.1/13.2.2) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`13.2.1` -> `13.2.2`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/13.2.1/13.2.2) | | [@angular/compiler-cli](https://github.com/angular/angular) | devDependencies | patch | [`13.2.1` -> `13.2.2`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/13.2.1/13.2.2) | | [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`13.2.1` -> `13.2.2`](https://renovatebot.com/diffs/npm/@angular%2fcore/13.2.1/13.2.2) | | [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`13.2.1` -> `13.2.2`](https://renovatebot.com/diffs/npm/@angular%2fforms/13.2.1/13.2.2) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`13.2.1` -> `13.2.2`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/13.2.1/13.2.2) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`13.2.1` -> `13.2.2`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/13.2.1/13.2.2) | | [@angular/router](https://github.com/angular/angular) | dependencies | patch | [`13.2.1` -> `13.2.2`](https://renovatebot.com/diffs/npm/@angular%2frouter/13.2.1/13.2.2) | --- ### Release Notes <details> <summary>angular/angular</summary> ### [`v13.2.2`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#​1322-2022-02-08) [Compare Source](angular/angular@13.2.1...13.2.2) ##### compiler | Commit | Type | Description | | -- | -- | -- | | [37af6abb49](angular/angular@37af6ab) | fix | allow banana-in-a-box bindings to end with non-null assertion ([#​37809](angular/angular#37809)) | ##### forms | Commit | Type | Description | | -- | -- | -- | | [b75e90f809](angular/angular@b75e90f) | fix | incorrectly keeping track of ngModel with ngFor inside a form ([#​40459](angular/angular#40459)) | ##### http | Commit | Type | Description | | -- | -- | -- | | [3fae6637e7](angular/angular@3fae663) | perf | remove IE special status handling ([#​44354](angular/angular#44354)) | ##### upgrade | Commit | Type | Description | | -- | -- | -- | | [b9aab0c87b](angular/angular@b9aab0c) | fix | Do not trigger duplicate navigation events from Angular Router ([#​43441](angular/angular#43441)) | #### Special Thanks Alan Agius, Alan Cohen, Andrew Kushnir, Andrew Scott, Daniel Díaz, Dario Piotrowicz, Doug Parker, Jayson Acosta, Joey Perrott, JoostK, Kristiyan Kostadinov, Olivier Capuozzo, Ramzan, Shai Reznik, TANMAY SRIVASTAVA, dario-piotrowicz, iRealNirmal, jhonyeduardo, mgechev and zuckjet <!-- CHANGELOG SPLIT MARKER --> </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1160 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
|
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.
… Router
This code mimics behavior that Google Analytics has been using to
prevent duplicate navigations. They set up their own
HybridRoutingServicelocation sync to avoid duplicate navigations that came from the Angular
router. This would happen because the Angular router would trigger a
navigation, which would then get picked up by the
$locationShim, whichwould trigger a
$locationChangeStart, which would then be picked up bythe
setUpLocationSyncwatcher here, which would again trigger anavigation in the Angular Router.
All of this can be prevented by checking if the
navigationIdexists onthe history state object. This property is added by the Angular router
during navigations.
fixes #21610