Skip to content

feat(router): add method to convert an Url to RouterStateSnapshot#30411

Closed
yelhouti wants to merge 1 commit intoangular:masterfrom
yelhouti:fix-15750
Closed

feat(router): add method to convert an Url to RouterStateSnapshot#30411
yelhouti wants to merge 1 commit intoangular:masterfrom
yelhouti:fix-15750

Conversation

@yelhouti
Copy link
Copy Markdown
Contributor

convert any Url to a routeStateSnapshot (if they exist).
This allows to check if user can access link before displaying it

fixes #15750

PR Checklist

Please check if your PR fulfills the following requirements:

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?

Enable to retrieve RouterStateSnapshot from URL

Issue Number: 15750

What is the new behavior?

Router allows to retrieve RouterStateSnapshot from Url

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

convert any url to a routeStateSnapshot (if they exist).
This allows to check if user can access link before displaying it

rerun CI
return urlTree;
}

/** Converts given Url to RouterStateSnapshot */
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/** Converts given Url to RouterStateSnapshot */
/** Converts given URL to `RouterStateSnapshot` */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also use some additional documentation. It should be noted that if you pass a URL to a lazy-loaded path, that module will actually get loaded by applyRedirects. We might want to think about these kinds of unintended consequences more before adding this feature.

Comment on lines +1021 to +1041
let resolve: any = null;
let reject: any = null;

const promise = new Promise<boolean>((res, rej) => {
resolve = res;
reject = rej;
});
const transition: NavigationTransition = {
...this.getTransition(),
id: 1,
source: 'imperative',
restoredState: null,
currentUrlTree: this.currentUrlTree,
currentRawUrl: this.rawUrlTree,
rawUrl: mergedTree, resolve, reject, promise,
currentSnapshot: this.routerState.snapshot,
currentRouterState: this.routerState,
extractedUrl: this.urlHandlingStrategy.extract(mergedTree)
};
const moduleInjector = this.ngModule.injector;
return of (transition)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let resolve: any = null;
let reject: any = null;
const promise = new Promise<boolean>((res, rej) => {
resolve = res;
reject = rej;
});
const transition: NavigationTransition = {
...this.getTransition(),
id: 1,
source: 'imperative',
restoredState: null,
currentUrlTree: this.currentUrlTree,
currentRawUrl: this.rawUrlTree,
rawUrl: mergedTree, resolve, reject, promise,
currentSnapshot: this.routerState.snapshot,
currentRouterState: this.routerState,
extractedUrl: this.urlHandlingStrategy.extract(mergedTree)
};
const moduleInjector = this.ngModule.injector;
return of (transition)
const moduleInjector = this.ngModule.injector;
return of ({extractedUrl: this.urlHandlingStrategy.extract(mergedTree)} as NavigationTransition)

I think this can be simplified quite a bit. The applyRedirects only uses extractedUrl from the transition and recognize only uses urlAfterRedirects which is set by applyRedirects.

Comment on lines +742 to +745

class ComponentA {}
class ComponentB {}
class ComponentC {}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you place these inside the describe instead?

@atscott atscott added target: major This PR is targeted for the next major release feature Label used to distinguish feature request from other issues action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Aug 6, 2020
@atscott atscott removed the request for review from a team August 6, 2020 20:02
describe('urlToRouteStateSnapshot', () => {
beforeEach(() => { TestBed.configureTestingModule({imports: [RouterTestingModule]}); });

it('should return snapshot or exception', inject([Router], async(r: Router) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would also probably be a good idea to cover the following scenarios as well:

  • loadChildren
  • canActivate
    • rejecting
    • returning UrlTree
  • redirectTo

@atscott
Copy link
Copy Markdown
Contributor

atscott commented Aug 6, 2020

Closing as duplicate of #15826

@atscott atscott closed this Aug 6, 2020
@angular-automatic-lock-bot
Copy link
Copy Markdown

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 Sep 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews area: router cla: yes feature Label used to distinguish feature request from other issues target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Retrieve all route segments configuration by URL

4 participants