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

types: add RouteMeta interface to enable module augmentation #3385

Merged
merged 3 commits into from Jun 21, 2021

Conversation

HoldYourWaffle
Copy link
Contributor

@HoldYourWaffle HoldYourWaffle commented Nov 24, 2020

Close #3183

I added a RouteMeta interface to the type declarations to allow for stronger type safety using module augmentation.

For example:

declare module 'vue-router/types/router' {
    interface RouteMeta {
        x: string;
    }
}

Would define myRoute.meta.x to be a string. Non-existent properties will show an error.

Unfortunately this is a (minor) breaking change. As far as I know this can't be avoided without sacrificing the added type-safety.
Users can manually revert to the original "any" behavior using an augmentation like this:

declare module 'vue-router/types/router' {
    interface RouteMeta {
        [key: string]: any;
    }
}

I can't make this the default however, because it can't be overwritten using module augmentation.
In other words: the "any"-index type would stay "active", thus defeating the purpose of added type-safety.

I hope this suggestion can be considered nonetheless.

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.

Thanks, can you make like the existing one at vuejs/router#407 and add a test?

@HoldYourWaffle
Copy link
Contributor Author

@posva Although better than using plain any, I don't think having RouteMeta extend Record<..., any> is the best way here.
Like I mentioned in my initial comment, this kind of defeats the purpose of the added type safety.

It's better than nothing of course, since properties that have been declared will have their appropriate types.
However, invalid (undeclared) properties will not cause an error, meaning that things like typo's will go unnoticed.

Therefore I'd propose leaving this "Record extension" up to the user. Then anyone who wants to is free to have their RouteMeta extend it, but it won't be forced on all the others.

@posva
Copy link
Member

posva commented Nov 24, 2020

For Vue Router 3, that would be a breaking change, so we will have to keep any, but it can maybe be changed to unknown in v4 (vue-router-next)

@HoldYourWaffle
Copy link
Contributor Author

I suppose that's the only way then. I'll update this as soon as I get back to my workspace.
Should I create a PR to vue-router-next as well for changing any to unknown?

@posva
Copy link
Member

posva commented Dec 1, 2020

Sure! Let's see if existing tests pass and if the change is reasonable

types/router.d.ts Outdated Show resolved Hide resolved
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.

The last thing that needs to be done here is to export RouteMeta from index.d.ts and add a test at https://github.com/vuejs/vue-router/tree/dev/types/test 🙂

@posva posva changed the title types: Add RouteMeta interface to enable module augmentation types: add RouteMeta interface to enable module augmentation Jun 21, 2021
@posva posva merged commit 260f737 into vuejs:dev Jun 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Route meta type to a user-extensible empty interfaces
2 participants