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

feat(types): NavigationGuardNext #2497

Merged
merged 3 commits into from
May 11, 2020
Merged

feat(types): NavigationGuardNext #2497

merged 3 commits into from
May 11, 2020

Conversation

beary
Copy link
Contributor

@beary beary commented Nov 25, 2018

Expose a Next type for this usage:

// ...
import { Route, Next } from 'vue-router'

@Component
export default class Verify extends Vue {
  // ...
  private beforeRouteEnter(to: Route, from: Route, next: Next) {
    next(vm => {
      vm.$data.username = decodeURIComponent(to.query.username || '')
    })
  }
}

@posva posva added the Typescript Typescript related issues label Nov 25, 2018
@kevlarr
Copy link

kevlarr commented Feb 17, 2019

Exactly the PR I was about to submit.. @posva How do PRs get reviewed/merged?

@posva
Copy link
Member

posva commented Feb 17, 2019

I will come at it when I have time, after other Bugs and features

@voool
Copy link

voool commented Mar 18, 2019

As a workaround, to grab the next interface with Typescript you can do

import { NavigationGuard } from 'vue-router';
type Next = Parameters<NavigationGuard>[2];

@kevlarr
Copy link

kevlarr commented Mar 19, 2019

Nice @voool, that's really clean! I didn't know that was possible - I've been copying the type def into my own code, which is more verbose and also more brittle (in case the def changes at all).

@andredewaard
Copy link

Any progress on when this will be merged?

@faustbrian
Copy link

@posva any progress on this?

@d2phap
Copy link

d2phap commented Apr 28, 2020

Any update?

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

I'm renaming the type to match vue-router-next
One thing that is problematic is that only beforeRouteEnter's next allows a callback. So I think we should export a type without the callback and make a specific type for beforeRouteEnter where we change next to include (vm: V) => any

@@ -7,10 +7,11 @@ type ErrorHandler = (err: Error) => void;
export type RouterMode = "hash" | "history" | "abstract";
export type RawLocation = string | Location;
export type RedirectOption = RawLocation | ((to: Route) => RawLocation);
export type Next<V extends Vue = Vue> = (to?: RawLocation | false | ((vm: V) => any) | void) => void;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export type Next<V extends Vue = Vue> = (to?: RawLocation | false | ((vm: V) => any) | void) => void;
export type NavigationGuardNext<V extends Vue = Vue> = (to?: RawLocation | false | ((vm: V) => any) | void) => void;

types/index.d.ts Outdated Show resolved Hide resolved
types/router.d.ts Outdated Show resolved Hide resolved
Copy link
Contributor Author

@beary beary left a comment

Choose a reason for hiding this comment

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

Looks good 😃

@beary beary requested a review from posva May 11, 2020 08:00
Copy link
Contributor Author

@beary beary left a comment

Choose a reason for hiding this comment

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

👍

@beary
Copy link
Contributor Author

beary commented May 11, 2020

I am not familiar with the operation of pr, if it interferes with people, I am sorry.
Or if the commit record is too confusing, you can close this pull request and create a new one.

@posva
Copy link
Member

posva commented May 11, 2020

No worries for the commit history @beary, we squash commits at the end! Let me know if this is things are not clear and I will finish this up

@beary
Copy link
Contributor Author

beary commented May 11, 2020

@posva
I read your changes, I think it is very good, And there is nothing I need to add!

@posva
Copy link
Member

posva commented May 11, 2020

@beary There is a change left: #2497 (review)

Copy link
Contributor Author

@beary beary left a comment

Choose a reason for hiding this comment

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

Reviewed

Copy link
Contributor Author

@beary beary left a comment

Choose a reason for hiding this comment

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

I submitted it for review, but it is still in a reviewable state

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

One thing that is problematic is that only beforeRouteEnter's next allows a callback. So I think we should export a type without the callback and make a specific type for beforeRouteEnter where we change next to include (vm: V) => any

@posva
Copy link
Member

posva commented May 11, 2020

@beary because the review is the same

Copy link
Contributor Author

@beary beary left a comment

Choose a reason for hiding this comment

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

Reviewed

@posva
Copy link
Member

posva commented May 11, 2020

what? see #2497 (review)
if you don't see where to add, that's fine, just let me know and I will pick it but let's not go in circles and waste each other time

@beary
Copy link
Contributor Author

beary commented May 11, 2020

@posva
Sorry I couldn't add it recently

@posva posva changed the title TypeScript: Expose a Next type feat(types): NavigationGuardNext May 11, 2020
@posva posva merged commit d18c497 into vuejs:dev May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typescript Typescript related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants