chore: add script setup tests #890
Conversation
tsconfig.json
Outdated
| "module": "esnext", | ||
| "lib": ["DOM", "ES2015", "ES2017"], | ||
| "strict": true, | ||
| "allowJs": true, |
There was a problem hiding this comment.
Added this since I think it makes sense to have some JS based <script setup> components - since we only use TS at the moment, which is not really representative of al the users.
Why is this breaking the build?
There was a problem hiding this comment.
I think it would be better to stick to TS components as this ensures vue-tsc can properly check them. And if it works in TS, it works in JS 🙂 So it would be better to remove this, and convert the script setup to TS I think.
There was a problem hiding this comment.
Thanks @lmiller1990
The lockfile received quite a few changes, maybe this is what caused the failing build?
A few changes are needed, ping when done if you want another review
| <script setup> | ||
| import { toRefs, ref, inject } from 'vue' | ||
| const props = defineProps({ |
There was a problem hiding this comment.
in TS, this can be written more elegantly using defineProps<{ title: string}>()
There was a problem hiding this comment.
Sure. Do you think there's a use case for including some plain JS tests, too? Ideally we should running tests against a wide variety of configs - not sure type safety is a big deal in these kind of fixtures.
There was a problem hiding this comment.
It would be nice to have integration tests as a safety net against various config indeed (something like a mini CLI project in TS and in JS, using the built version of VTU). But I think for our unit tests we should be fine if sticking to TS: it's pretty much the hardest thing to get right from our tooling perspective.
| } | ||
| }) | ||
|
|
||
| console.log(wrapper.html()) |
tsconfig.json
Outdated
| "module": "esnext", | ||
| "lib": ["DOM", "ES2015", "ES2017"], | ||
| "strict": true, | ||
| "allowJs": true, |
There was a problem hiding this comment.
I think it would be better to stick to TS components as this ensures vue-tsc can properly check them. And if it works in TS, it works in JS 🙂 So it would be better to remove this, and convert the script setup to TS I think.
|
@cexbrayat thanks for the review, updated the PR. |
|
Awesome, thanks Lachlan! |
2 participants
Added some tests based on the feedback in #876