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

Redirect if a guard returns a UrlTree #26521

Closed
wants to merge 7 commits into
base: master
from

Conversation

@jasonaden
Contributor

jasonaden commented Oct 17, 2018

Fixes #24618

@jasonaden jasonaden requested a review from benlesh Oct 17, 2018

@googlebot googlebot added the cla: yes label Oct 17, 2018

@mary-poppins

This comment has been minimized.

mary-poppins commented Oct 17, 2018

@benlesh

Only one small change I'd make.

of ({
guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts())
} as Partial<NavigationTransition>)
.pipe(checkGuardsOperator(injector))
.subscribe(t => check(!!t.guardsResult), (e) => { throw e; });
.subscribe(

This comment has been minimized.

@benlesh

benlesh Oct 17, 2018

Contributor

RxJS core team recommends using an Observer for all subscriptions that use more than one callback, like:

obs.subscribe({
  next(value) { },
  error(err) { }
});

This comment has been minimized.

@gkalpak

gkalpak Oct 17, 2018

Member

OOC, why?

This comment has been minimized.

@jasonaden

jasonaden Oct 17, 2018

Contributor

Convention and more readable.

Show resolved Hide resolved packages/router/src/utils/type_guards.ts Outdated
@mary-poppins

This comment has been minimized.

mary-poppins commented Oct 17, 2018

@mary-poppins

This comment has been minimized.

mary-poppins commented Oct 17, 2018

jasonaden added some commits Oct 3, 2018

feat(router): allow redirect from guards by returning UrlTree
* Improve type checking within the CheckGuards function
* Change public API to allow returning of UrlTree instead of boolean
feat(router): allow guards to return UrlTree as well as boolean
* Removed `andObservable` helper function in favor of inline implementation
* Flow `boolean | UrlTree` through guards check
* Add tests to verify behavior of `checkGuards` function flowing `UrlTree` properly
@jasonaden

This comment has been minimized.

Contributor

jasonaden commented Oct 17, 2018

@jasonaden jasonaden requested a review from IgorMinar Oct 18, 2018

@jasonaden

This comment has been minimized.

Contributor

jasonaden commented Oct 18, 2018

@sarunint

Great feature!

Show resolved Hide resolved packages/router/src/interfaces.ts Outdated
Show resolved Hide resolved packages/router/src/interfaces.ts Outdated
Show resolved Hide resolved packages/router/src/interfaces.ts Outdated
Show resolved Hide resolved packages/router/src/interfaces.ts
@mary-poppins

This comment has been minimized.

mary-poppins commented Oct 30, 2018

@mary-poppins

This comment has been minimized.

mary-poppins commented Oct 30, 2018

@ngbot ngbot bot added this to the needsTriage milestone Oct 31, 2018

@matsko matsko closed this in 9f4299d Oct 31, 2018

matsko added a commit that referenced this pull request Oct 31, 2018

matsko added a commit that referenced this pull request Oct 31, 2018

feat(router): allow redirect from guards by returning UrlTree (#26521)
* Improve type checking within the CheckGuards function
* Change public API to allow returning of UrlTree instead of boolean

PR Close #26521

matsko added a commit that referenced this pull request Oct 31, 2018

feat(router): allow guards to return UrlTree as well as boolean (#26521)
* Removed `andObservable` helper function in favor of inline implementation
* Flow `boolean | UrlTree` through guards check
* Add tests to verify behavior of `checkGuards` function flowing `UrlTree` properly

PR Close #26521

matsko added a commit that referenced this pull request Oct 31, 2018

sculove added a commit to sculove/angular that referenced this pull request Nov 2, 2018

sculove added a commit to sculove/angular that referenced this pull request Nov 2, 2018

sculove added a commit to sculove/angular that referenced this pull request Nov 2, 2018

sculove added a commit to sculove/angular that referenced this pull request Nov 2, 2018

feat(router): allow redirect from guards by returning UrlTree (angula…
…r#26521)

* Improve type checking within the CheckGuards function
* Change public API to allow returning of UrlTree instead of boolean

PR Close angular#26521

sculove added a commit to sculove/angular that referenced this pull request Nov 2, 2018

feat(router): allow guards to return UrlTree as well as boolean (angu…
…lar#26521)

* Removed `andObservable` helper function in favor of inline implementation
* Flow `boolean | UrlTree` through guards check
* Add tests to verify behavior of `checkGuards` function flowing `UrlTree` properly

PR Close angular#26521

sculove added a commit to sculove/angular that referenced this pull request Nov 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment