Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Rename Router.navigateByUrl #56672

Open
Ghostbird opened this issue Jun 26, 2024 · 0 comments
Open

Proposal: Rename Router.navigateByUrl #56672

Ghostbird opened this issue Jun 26, 2024 · 0 comments
Labels
area: router P5 The team acknowledges the request but does not plan to address it, it remains open for discussion
Milestone

Comments

@Ghostbird
Copy link

Ghostbird commented Jun 26, 2024

Which @angular/* package(s) are relevant/related to the feature request?

router

Description

I've been extremely baffled trying to fix a bug in our app where routing just wouldn't work.

It turns out the Router.navigateByUrl is apparently misnamed. Of course the real functionality is completely correctly documented. I'm at fault for not checking the documentation earlier. However in my defence, the realisation that this method does something completely different than its name suggests, meant that I didn't realise that it was this function that caused the problem for several hours. Once I realised that, I checked the documentation, and immediately understood the issue.

Router.navigateByUrl does not navigate to a URL1. It instead routes to the given Angular route string.

I was trying to restore an older page the user visited before navigating away, and was trying to do so by doing this:

// Before navigate away:
sessionStorage.setItem('returnUri', window.location.href);

// After return
this.router.navigateByUrl(sessionStorage.getItem('returnUri') ?? window.location.origin);

The result is that you always get routed to the fallback route, since navigateByUrl cannot navigate to URLs.

Proposed solution

  • Mark navigateByUrl as deprecated.
  • Create a function routeToAbsolutePath or just routeToPath (name pending).
  • Both the old and new function definitions should run the same logic.
  • Really remove navigateByUrl after some major versions.

I expect the deprecation marker will probably help people a lot already. My IDE automatically complains when I use a deprecated method. Then I'll read the documentation immediately, to find out why it's deprecated, and which function I should use instead.

Alternatives considered

Don't do anything. It's not actually broken. Just confusingly named, but it's been that way for a long time.

Footnotes

  1. Since the term URL is sometimes unclear, in the context of the Angular Router I'd understand a URL as something that conforms to the URI grammar below where the scheme value is https. Given the context I've excluded otherwise valid URI schemes that would also create valid URLs, such as mailto, wss, tel etc.
    URI = scheme ":" ["//" authority] path ["?" query] ["#" fragment]

@ngbot ngbot bot added this to the needsTriage milestone Jun 26, 2024
@atscott atscott added P5 The team acknowledges the request but does not plan to address it, it remains open for discussion labels Jun 26, 2024
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: router P5 The team acknowledges the request but does not plan to address it, it remains open for discussion
Projects
None yet
Development

No branches or pull requests

3 participants