Skip to content

Conversation

vmuresanu
Copy link

@vmuresanu vmuresanu commented Nov 7, 2022

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

With version 14's standalone API createRouter now will have a new function config withLocationStrategy that we can set whether we want to use hash or not. If not provided the default is Path Location Strategy

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@pullapprove pullapprove bot requested a review from atscott November 7, 2022 13:29
@vmuresanu vmuresanu force-pushed the add-location-strategy-to-provide-router-configs branch from 5805cb5 to 72d997c Compare November 7, 2022 13:30
@atscott
Copy link
Contributor

atscott commented Nov 7, 2022

This is intended. If you want to use HashLocationStrategy, you can add your own provider: {provide: LocationStrategy, useClass: HashLocationStrategy}. Using hash location is quite rare and we would rather simplify the API surface of the Router than provide, document, and maintain a function that is syntax sugar for a 1-liner that less than 1% of Angular apps use.

@atscott atscott closed this Nov 7, 2022
@vmuresanu
Copy link
Author

@atscott I agree that Hash location strategy is a one liner but in my opinion because it relates to Router, it should be included at least as an interface property of RouterConfigOptions.

First of all it is not documented at all that using Angular 14.2's provideRouter, useHash has been dropped from configuration and we have to import it as a generic provider {provide: LocationStrategy, useClass: HashLocationStrategy}. At least not that I am aware of, and trust me it took me a whole day switching between Angular documentation, searching PR's and opening the source code of router realising that it has been dropped.

Let assume that we also have paramsInheritanceStrategy property
For instance this is how it was with RouterModule:

providers: [
  importProvidersFrom(
    RouterModule.forRoot(appRoutes, { useHash: true, paramsInheritanceStrategy: 'always' }),
    ...
  )
]

And this is how we have to do using provideRouter:

providers: [
   provideRouter(appRoutes, withRouterConfig({ paramsInheritanceStrategy: 'always' })),
   importProvidersFrom(
    ...
   { provide: LocationStrategy, useClass: HashLocationStrategy },
  )
]

I don't know about your opinion, but if you ask me, first way makes more sense as useHash relates to router config.

@atscott
Copy link
Contributor

atscott commented Nov 10, 2022

@vmuresanu Technically the hash strategy is a location configuration in @angular/common and you can use it outside of the Router or with any custom router implementations. So while the net effect of using the router is that it updates the URL which is controlled by the strategy, I don't totally agree. The location strategy is an application-wide configuration and affects anything that imports Location. Of course, the Router does this in a few places but plenty of other things might as well.

I do think it becomes confusing when there are multiple ways to do things that seem just about the same. "Why useHash: true the provider seems works? Do I still need useHash: true? What if I use both?" In this case, these two different ways also appear to work on separate packages (@angular/router and @angular/common). The useHash API also hides and makes the the relation between PlatformLocation and LocationStrategy more confusing. There's definitely a balance between clarity and brevity to be found.

At the very least, I do agree with you on the documentation. Would you like to send over a PR to update the page here? https://angular.io/guide/router

PS: your example does not need importProvidersFrom.

@vmuresanu
Copy link
Author

vmuresanu commented Nov 10, 2022

Understood, thanks for the detailed explanation. Sure, I would like to make a PR regarding updating the page. Will link it here as well when it is ready.

@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants