-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
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): prefer normal component over functional one #7687
Conversation
47ad14f
to
dae2958
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also reorder Vue.component
overload, shouldn't we?
Hmm, this reorder is not cost free. It will break functional component's type inference: |
Oh, I didn't notice the functional component's inference will be broken. Hmm, I'm not sure we should merge this change, then... After this change, the prop types of functional component will not be checked, will they? I don't know how many users rely on it but I personally rely on |
No, they will not. I can change it to be checked. However, normal component's render function will still have non-existing context argument in types. |
IMHO, allowing non-existing context in normal component is acceptable since the users probably not want to check it by type checker. But props should be checked since users probably change props' name/type during maintenance and want to use type checker to help the refactoring. |
update ComponentOptions apply change to component
dae2958
to
fa04f0a
Compare
@ktsn do you think this is ready to merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
Other information:
Background:
vuejs/vetur#676
vuejs/vetur#627 (comment)
TypeScript cannot give reasonable completion for
Vue.extend
. For example,methods
,data
will not appear in completion list. This is caused by the intricate overloading resolution in compiler host. microsoft/TypeScript#8005.This problem will probably not fixed in foreseeable future. So we can work around this by placing normal component option before functional component. In most case, normal component option provides better completion. However, this will break type inference for functional component definition.
By providing an additional argument in normal component's
render
, we can at least compile functional component. Unfortunately, this work around will skip checkingRenderContenxt
. IMHO Vue's type checking isn't primarily for checking but for completion and definition looking up. So I think this is a advantageous trade-off.