Skip to content

feat(router): Allow Route.redirectTo to be a function which returns a… #52606

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

Closed
wants to merge 2 commits into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Nov 7, 2023

… string or UrlTree

This commit updates the logic around Route.redirectTo to enable using a function to create the redirect. This function can return a string, ands acts the same as previous string redirects, or a UrlTree, which will act as an absolute redirect.

To be useful, the redirect function needs access to the params and data. Today, developers can access these in their redirect strings, for example {path: ':id', redirectTo: '/user/:id'}. Unfortunately, developers only have access to params and data on the current route today in the redirect strings. The params and data in the RedirectFn give developers access to anything from the matched parent routes as well. This is done as the same way as param and data aggregation later on (

export function getInherited(
route: ActivatedRouteSnapshot, parent: ActivatedRouteSnapshot|null,
paramsInheritanceStrategy: ParamsInheritanceStrategy = 'emptyOnly'): Inherited {
let inherited: Inherited;
const {routeConfig} = route;
if (parent !== null &&
(paramsInheritanceStrategy === 'always' ||
// inherit parent data if route is empty path
routeConfig?.path === '' ||
// inherit parent data if parent was componentless
(!parent.component && !parent.routeConfig?.loadComponent))) {
inherited = {
params: {...parent.params, ...route.params},
data: {...parent.data, ...route.data},
resolve: {
// Snapshots are created with data inherited from parent and guards (i.e. canActivate) can
// change data because it's not frozen...
// This first line could be deleted chose to break/disallow mutating the `data` object in
// guards.
// Note that data from parents still override this mutated data so anyone relying on this
// might be surprised that it doesn't work if parent data is inherited but otherwise does.
...route.data,
// Ensure inherited resolved data overrides inherited static data
...parent.data,
// static data from the current route overrides any inherited data
...routeConfig?.data,
// resolved data from current route overrides everything
...route._resolvedData,
}
};
} else {
inherited = {
params: route.params,
data: route.data,
resolve: {...route.data, ...(route._resolvedData ?? {})}
};
}
if (routeConfig && hasStaticTitle(routeConfig)) {
inherited.resolve[RouteTitleKey] = routeConfig.title;
}
return inherited;
}
). In order to accomplish this, we inherit params and data while matching, after the ActivatedRouteSnapshot is created for the matched route rather than waiting until the end.

The RedirectFunction does not return the full
ActivatedRouteSnapshot interface. Some things are not accurately known at the route matching phase. For example, resolvers are not run until later, so any resolved title would not be populated. The same goes for lazy loaded components. The is also true for all the snapshots up to the root, so properties that include parents (root, parent, pathFromRoot) are also excluded. And naturally, the full route matching hasn't yet happened so firstChild and children are not available either.

fixes #13373
resolves #28661 (though not for the redirect string - you would return a UrlTree from the function)

@atscott atscott added the target: minor This PR is targeted for the next minor release label Nov 7, 2023
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Nov 7, 2023
@ngbot ngbot bot added this to the Backlog milestone Nov 7, 2023
@atscott atscott force-pushed the redirectFunction branch 3 times, most recently from 0ee8cbc to 62c3106 Compare November 13, 2023 17:17
@atscott atscott added target: major This PR is targeted for the next major release breaking changes and removed target: minor This PR is targeted for the next minor release labels Nov 14, 2023
@atscott
Copy link
Contributor Author

atscott commented Nov 14, 2023

Updating target to major. This will likely need to be considered a breaking change. Expanding the type on the redirectTo results in build failures because places looking at the route config and expecting it to be a string will not work without a migration.

@angular-robot angular-robot bot added area: docs Related to the documentation area: build & ci Related the build and CI infrastructure of the project labels Feb 16, 2024
@angular-robot angular-robot bot removed area: docs Related to the documentation area: build & ci Related the build and CI infrastructure of the project labels Feb 16, 2024
@atscott atscott force-pushed the redirectFunction branch 2 times, most recently from a4322b7 to a9b79ac Compare February 20, 2024 18:14
@angular-robot angular-robot bot added the detected: breaking change PR contains a commit with a breaking change label Feb 20, 2024
@atscott atscott marked this pull request as ready for review February 20, 2024 18:14
@AndrewKushnir AndrewKushnir modified the milestones: Backlog, v18-candidates Feb 20, 2024
Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

@pullapprove pullapprove bot requested a review from dylhunn February 21, 2024 15:06
@atscott atscott added the action: global presubmit The PR is in need of a google3 global presubmit label Mar 12, 2024
@atscott atscott force-pushed the redirectFunction branch 2 times, most recently from 7992b66 to 53790be Compare March 12, 2024 16:30
… string or UrlTree

This commit updates the logic around `Route.redirectTo` to enable using a
function to create the redirect. This function can return a string,
ands acts the same as previous string redirects, or a `UrlTree`, which
will act as an absolute redirect.

To be useful, the redirect function needs access to the params and data.
Today, developers can access these in their redirect strings, for
example `{path: ':id', redirectTo: '/user/:id'}`. Unfortunately,
developers only have access to params and data on the _current route_
today in the redirect strings. The params and data in the `RedirectFn`
give developers access to anything from the matched parent routes as
well. This is done as the same way as param and data aggregation later
on (https://github.com/angular/angular/blob/897f014785578d87bc655ea6ae9e113653960f50/packages/router/src/router_state.ts#L236-L278).
In order to accomplish this, we inherit params and data while
matching, after the `ActivatedRouteSnapshot` is created for the matched
route rather than waiting until the end.

The `RedirectFunction` does not return the full
`ActivatedRouteSnapshot` interface. Some things are not accurately known
at the route matching phase. For example, resolvers are not run until
later, so any resolved title would not be populated. The same goes for lazy
loaded components. The is also true for all the snapshots up to the
root, so properties that include parents (root, parent, pathFromRoot)
are also excluded. And naturally, the full route matching hasn't yet
happened so firstChild and children are not available either.

fixes angular#13373
resolves angular#28661 (though not for the redirect string - you would return a
`UrlTree` from the function)

BREAKING CHANGE: This change allows `Route.redirectTo` to be a function
in addition to the previous string. Code which expects `redirectTo` to
only be a string on `Route` objects will need to be adjusted.
@atscott atscott force-pushed the redirectFunction branch 2 times, most recently from 08e0f2c to 4a846ed Compare March 14, 2024 16:30
@atscott
Copy link
Contributor Author

atscott commented Mar 14, 2024

@atscott atscott removed the action: global presubmit The PR is in need of a google3 global presubmit label Mar 14, 2024
@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Mar 14, 2024
@atscott
Copy link
Contributor Author

atscott commented Mar 14, 2024

This PR was merged into the repository by commit 2b80258.

@atscott atscott closed this in 2b80258 Mar 14, 2024
@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 Apr 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: router breaking changes detected: breaking change PR contains a commit with a breaking change detected: feature PR contains a feature commit target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preserve query params in redirectTo for absolute paths Route redirectTo as function / class
3 participants