Consecutive router navigations with "replaceUrl: true" will pollute browser history

[agrossman]

I'm submitting a ...

[ ] Regression (behavior that used to work and stopped working in a new release)
[X] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request

Current behavior

Navigating multiple times with router.navigate and specifying replaceUrl: true will cause a browser history entry to be created for each navigation that occurs before the first navigation has completed.

Expected behavior

Specifying replaceUrl: true should always replace the current URL (location.replace). It should never perform a standard navigation (href/hash manipulation).

Minimal reproduction of the problem with instructions

Example here: http://plnkr.co/edit/UGUnIU?p=preview

Steps:

• Open in a standalone window
• Click the button a few times
• Observe the additional history entries in the Back button pulldown menu
• You can also increase the number of iterations to prove that this will add a new history entry for each consecutive navigation, not only the first.
• If you reduce the loop count to 1 iteration, no history is generated (as expected).

What is the motivation / use case for changing the behavior?

• It's wrong according to its own spec.
• A common use case is for apps to track various bits of state in the URL hash, and different components may track their own state independently. This is generally not a problem as we have queryParamsHandling: 'merge'. However, the spurious history entries can break some use cases.

Please tell us about your environment

Angular version: 4.2.5

Browser:
- [X] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX

Analysis

I've looked at the source and I'm pretty sure the problem is in the runNavigate function:

  private runNavigate(
      url: UrlTree, rawUrl: UrlTree, shouldPreventPushState: boolean, shouldReplaceUrl: boolean,
      id: number, precreatedState: RouterStateSnapshot|null): Promise<boolean> {
    if (id !== this.navigationId) {
      this.location.go(this.urlSerializer.serialize(this.currentUrlTree));
      this.routerEvents.next(new NavigationCancel(
          id, this.serializeUrl(url),
          `Navigation ID ${id} is not equal to the current navigation id ${this.navigationId}`));
      return Promise.resolve(false);
    }
    // ...
  }

If the ID doesn't match (which will always be the case if multiple navigations happen in rapid succession) then it will simply ignore the shouldReplaceUrl parameter entirely and perform a normal navigation.

[CodeJason]

As a temporary workaround, you can just wrap your navigate statements in a setTimeout.
Assuming that you're using the replaceUrl to store state, it shouldn't affect the user experience too much.

[jd-carroll]

@jasonaden Does the hypothesis sound accurate? We are facing this issue in a particular use case and it would be good to get a fix in. Happy to help develop test cases / fixes to move this along (provided the hypothesis seems reasonable)

[CodeJason]

@jd-carroll If you take a look at this fork of the current issue: http://plnkr.co/edit/UGls54?p=preview
You can see there is no difference in behavior between setting the original plunk navigation loop count to 1 and setting my fix to any number.