Added Option to Automatically Add Vitest to New Projects using svelte-create

[bertybot]

Issue

Getting started with unit testing in Sveltekit can be confusing, which was brought up in this issue here #4423

Resolution

• Added the ability to automatically add Vitest with example tests to new projects using the svelte-create package. This should help people new to unit tests in Sveltekit get started with reasonable defaults by scaffolding the configuration for them, and giving them example tests to start out with.

Testing

• Manually tested this with every config to make sure it worked.

Possible problems

• I have not tested this Vitest config when emulating Sveltekit specific behavior such as $app/env and $app/stores I might need input from @benmccann on this one...

[changeset-bot]

🦋 Changeset detected

Latest commit: 01ab4ba

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
create-svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[benmccann]

What is it that requires +vitest+playwright+typescript setup? Is there something in the Playwright config that needs to change?

[dominikg]

Do we want to add c8 + coverage script by default?
I think vitest asks you to install c8 if you run --coverage without it. Adding it by default means every user gets it, even if they are not interested in it.

[bertybot]

Do we want to add c8 + coverage script by default?

I think vitest asks you to install c8 if you run --coverage without it. Adding it by default means every user gets it, even if they are not interested in it.

Yea, this is true. Do you think I should just remove the coverage script as well?

[dominikg]

I'd skip it until we have an idea/documentation to actually build a full coverage test suite, which may need an additional util for mocking kit things as required.

Eg. hooks, page-endpoints and maybe even some parts of .svelte files are very difficult to test right now.

So a coverage report by c8 would be very inaccurate, either by giving a false sense of security when showing 100% even though it's not seeing everything, or by showing 50% and the user has no way of getting a higher coverage because tools are missing to implement tests.

[bertybot]

I'd skip it until we have an idea/documentation to actually build a full coverage test suite, which may need an additional util for mocking kit things as required.

Eg. hooks, page-endpoints and maybe even some parts of .svelte files are very difficult to test right now.

So a coverage report by c8 would be very inaccurate, either by giving a false sense of security when showing 100% even though it's not seeing everything, or by showing 50% and the user has no way of getting a higher coverage because tools are missing to implement tests.

Removed coverage tests

[dominikg]

Another discussion ;) globals: true would mean we'd have to also provide additional env typing for the test files, just leave it off and resort to imports? It's cleaner anyways

[bertybot]

Yea, just gonna admit that a lot of my decisions, have been based off what I've seen other people doing, and what my team has been doing.

I took a lot of inspiration from @davipon on his add vitest library
https://github.com/davipon/svelte-add-vitest/blob/main/templates/config/vite.config.js

I have yet to see someone not use globals I think because having to add them for every single test starts to get annoying, adding the imports just becomes repetitive boilerplate after a while. Definitely a preference though, the script already adds the vitest types to the tsconfig or Jsconfig based on what you choose.

[bertybot]

I pushed some updates, so this should hopefully be close to being able to be merged. The one remaining item is @dominikg's comments about global types: #5708 (review). Assuming we want to use globals, perhaps someone more familiar with TypeScript could add the types

THANKS @benmccann!!!

If its a huge problem then I think we should just leave off globals for now as @dominikg said it's cleaner anyway.

[bertybot]

removed the vitest globals in favor of named imports as suggested by @dominikg

while doing this I took the time to extensively test every configuration I found a bug adding two vite configs to every project which I fixed by removing the -vitest vite.config and renaming the +vitest vite.config.js to vite.config.ts

@benmccann I also fixed a few type errors that broke the Game test that you should check out. I just had to add a default property to the constructor and split the string out in to a char array in the test. These work but, they were kind of quick fixes so feel free to update to something more your style.

[dominikg]

I don't recommend using vite.config.ts . The way vite creates a temporary vite.config.hash.js isn't great imho.

[dominikg]

If we suggest colocating test files, there should be documentation about how to handle + prefixed files.
See #7121

[bertybot]

I don't recommend using vite.config.ts . The way vite creates a temporary vite.config.hash.js isn't great imho.

Updated and renamed vite.config.ts to vite.config.js and tested.

[dominikg]

unit test files are colocated in src, do we need to update tsconfig.json or show how that works at least so that users can have different ts settings for unit vs app code? or should the move it out of src in that case?

I'd really love to avoid going down the multiple tsconfig files road but i'm not sure you can have both playwright and vitest .spec.ts files with different gobal needs from a single tsconfig

[benmccann]

@dominikg why would you need different TypeScript settings for unit vs app code?

[dominikg]

@dominikg why would you need different TypeScript settings for unit vs app code?

eg node env vs not

[benmccann]

Isn't that just defined once per config? If you want multiple, I think you have to have multiple configs regardless of what directories things live in? I don't have a ton of experience writing tests in TypeScript projects, so I'm not understanding what alternative is being proposed and if there's some best practice around this that you think this PR might be violating?

[dominikg]

it depends, for basic unit tests sharing the same tsconfig works. but I've seen multiple projects splitting it into a separate tsconfig.test.json with include and exclude. due to the way kit generates the root tsconfig that could become difficult.

we can start with this setup but advanced usecases will require additional setup which isn't clear and there is no docs about it.

[dominikg]

is vitest-dev/vitest#2298 a blocker here? not supporting these makes some unit tests harder/ impossible

related: #6259

[bertybot]

is vitest-dev/vitest#2298 a blocker here? not supporting these makes some unit tests harder/ impossible

related: #6259

I think we agreed to basic unit testing to start giving the ability to configure to their projects needs as it grows.

This issue seems semi problematic but, could we not use mocks to get around this? I know in the issue the they use mockImport() which automatically makes a mock but couldn't for now they just use .mock() and do it by hand does that not work?

[jorgebv]

One late note since I saw the topic of globals was already discussed:

Some libraries enable features automatically based on these globals and users coming from jest may be confused as to why things aren’t working the same way out of the box. One that got me was Testing Library, which enables auto cleanup only if the global is present. https://testing-library.com/docs/react-testing-library/api#cleanup (this is a link to react docs but it applies equally to the svelte version of the library)

I personally have globals enabled but I have a second tsconfig to add the global types to my testing folder, which I saw you are trying to avoid. I don’t think there’s one true answer here but just wanted to leave this note in case it benefits anyone.

[Rich-Harris]

thanks all! very happy to get this in

[bertybot]

Thanks everyone!!!