replaceUrl not preserved in redirect

[stephenbunch]

Which @angular/* package(s) are the source of the bug?

router

Is this a regression?

Yes

Description

If I call router.navigate with replaceUrl: true, and if I have a CanActivateFn that returns a UrlTree, the resulting redirect loses the replaceUrl setting.

The relevant code is here:

angular/packages/router/src/router.ts

Lines 196 to 201 in e3a6bf9

	// The URL is already updated at this point if we have 'eager' URL
	// updates or if the navigation was triggered by the browser (back
	// button, URL bar, etc). We want to replace that item in history
	// if the navigation is rejected.
	replaceUrl: this.urlUpdateStrategy === 'eager' ||
	isBrowserTriggeredNavigation(currentTransition.source)

I can fix the issue by using replaceUrl setting if it's defined:

const extras = {
  // ...
  replaceUrl: currentTransition.extras.replaceUrl ?? (this.urlUpdateStrategy === 'eager' ||
  isBrowserTriggeredNavigation(currentTransition.source))
};

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/stackblitz-starters-ih775l?file=src%2Fmain.ts

Please provide the exception or error you saw

In the demo app I provided, when you click "Replace: Baz -> Bar", you should see "replace: /bar" in the history, but instead you just see "/bar".

Please provide the environment you discovered this bug in (run ng version)

"dependencies": {
  "rxjs": "7.8.1",
  "tslib": "2.6.2",
  "zone.js": "0.14.2",
  "@angular/core": "17.0.6",
  "@angular/forms": "17.0.6",
  "@angular/common": "17.0.6",
  "@angular/router": "17.0.6",
  "@angular/compiler": "17.0.6",
  "@angular/animations": "17.0.6",
  "@angular/platform-browser": "17.0.6"
},

Anything else?

No response

[atscott]

Indeed, this is also true of all the other properties in NavigationBehaviorOptions. Based on the comment here #27148 (comment), it seems like this was considered but not done. This was before my time as the router caretaker. I noticed this when adding the info and browserUrl properties and decided the properties should be carried over by default.

Changing this behavior would need to land in a major version. We would also need to do some preliminary tests to determine how breaking it is and decide if the change is worth making based on the results.

[atscott]

I ran tests inside google and found that updating replaceUrl value to be carried over for the redirect passed all tests. This change should be easy to land for v18.

[blathers16]

Hi, @atscott I have pushed a pull request for this, please let me know if it needs any changes.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}