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

fix(types): contravariant generic default in ComponentOption #7369

Merged
merged 1 commit into from
Jan 3, 2018

Conversation

HerringtonDarkholme
Copy link
Member

What kind of change does this PR introduce? (check at least one)

  • Bugfix

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

Other information:

Fix bugs spotted in vuejs/vue-test-utils#317 (comment).

Some type theory shenanigans below:

This pull request allows assigning ComponentOption of a Vue subclass to Component. The problem is that we used to mark default vm type as Vue in Component. However, since ComponentOption is contravariant against vm type V. The default bound should not be the upper bound Vue, but the lower bound never.

For a more concrete example, consider this usage:

interface MyComponent extends Vue {
   num: number;
 }
 
const option: ComponentOptions<MyComponent> = {
  // a lifetime method
  created(this: MyComponent) { return this.num.toFixed() }
}

Then, assigning option to ComponentOption<Vue>, we could in theory do unsafe operation:

const anotherOption: ComponentOptions<Vue> = option

anotherOption.created.call(new Vue) // error! no property num on Vue!

Though it is not how we really use Vue, type checker cannot know this. To workaround, we can mark anotherOption to have vm type as never. never is the bottom type of any type, so ComponentOption<never>'s create method can accept nothing, and thusly any create method from subclass can satisfy accepting nothing semantic. In practical words, this means we promise that we never use created like the way above.

Copy link
Member

@ktsn ktsn left a comment

Choose a reason for hiding this comment

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

Should we also change AsyncComponent default generic type?

@HerringtonDarkholme
Copy link
Member Author

Updated!

Copy link
Member

@ktsn ktsn left a comment

Choose a reason for hiding this comment

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

Thanks!

@yyx990803 yyx990803 merged commit 6ee6849 into vuejs:dev Jan 3, 2018
lovelope pushed a commit to lovelope/vue that referenced this pull request Feb 1, 2018
f2009 pushed a commit to f2009/vue that referenced this pull request Jan 25, 2019
aJean pushed a commit to aJean/vue that referenced this pull request Aug 19, 2020
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.

3 participants