-
Notifications
You must be signed in to change notification settings - Fork 667
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
#1526: Fixed missing import of the Cheerio type #1527
Conversation
Are you sure this is necessary? I am not seeing this typing error in |
Probably, you have Cheerio registered globally somehow. |
@unicrus works fine for me, can you run If that doesn't work, can you check if you have the file EDIT: If still doesn't work, please remove |
@lmiller1990, @pikax please take a look at the example to reproduce the bug: https://github.com/unicrus/vue-server-test-utils-1526. TypeScript is a strictly typed language, all types must be defined before usage. |
Your repo is overriding the "types": [
"@types/node"
] Making typescript only load those types! As per the documentation
If you remove the types option, it will pick up the types by default. Or you can add Cheerio to your types
|
That's right. But @vue/test-utils does not rely on global types for everything else, like Vue or Vuex. They're explicitly imported. Why should Cheerio be different? It's not a well-known type. |
I cannot build my TS project because of this issue. so I was forced to do this and rewrite my tests... |
@pikax any down sides to merge this types PR? I don't know TS super well, seems like it's blocking a few people - I don't see any downsides to the extra import (right?) |
Sorry, got side tracked and this went under the radar. My requested changes are not required, is just less code, if you want you can merge this. The explanation is, cheerio types don't expose a default import, but when you import with typescript it expects to have the default export, the flag on the the script tells typescript to basically do |
@unicrus if you can make those changes we can get this merged and released - we have a release in a day or two. |
@lmiller1990, done. 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.fix #xxx[,#xxx]
, where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information: