-
-
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): async Component types #11906
Conversation
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.
Can you add a test?
What exactly should be tested? - As I understand the repro there are no type tests. I just found https://github.com/vuejs/vue/blob/dev/test/unit/features/component/component-async.spec.js#L322 I can add there a test that the second example doesn't work (like https://codepen.io/mathe42/pen/poNNNYx with check that 'component resolved' is not found in document) |
In this case a type test case would suffice: https://github.com/vuejs/vue/tree/dev/types/test |
Thanks didn't saw that. Just pushed some tests (also for the simpe usages of AsyncComponent). Not shure about my |
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.
Thanks!
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.
Can you leave all unrelated changes out of the PR please? I think we will handle the formatting in a different commit
Yes sorry, there was more in the commits than I wanted. Working on it. |
@posva Removed the styling changes. Hope this is fine now. |
I repleced the Vue.component tests with a single one to test if it accepts AsyncComponents... |
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.
Thanks!
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
dev
branch for v2.x (or to a previous version branch), not themaster
branchfix #xxx[,#xxx]
, where "xxx" is the issue number)Other information:
Look at this codeexample:
The first example is like in the docs https://vuejs.org/v2/guide/components-dynamic-async.html#Handling-Loading-State and the second example should not work (I also looked at the implementation and I think this would not work).
The types are currently like such that the first example throws a type Error and the second throws no error. This PR fixes that.