Skip to content
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

Removed deprecated and updated vitest #74

Closed
wants to merge 1 commit into from
Closed

Removed deprecated and updated vitest #74

wants to merge 1 commit into from

Conversation

niktek
Copy link
Contributor

@niktek niktek commented Aug 11, 2022

This seems to be clearing all the fresh install issues for me.

@niktek niktek mentioned this pull request Aug 11, 2022
@endigo9740
Copy link
Contributor

I had suspected the same on this, but unfortunately your changes causes a lot of errors in the test runner. Almost half the tests fail to load after this change:

Screen Shot 2022-08-13 at 2 14 33 PM

Screen Shot 2022-08-13 at 2 15 44 PM

Again, I do believe it should be possible to drop this extra dep. In fact, when we implemented Vitest it was early days, and very rough around the edges (back in February). But they've made a lot of progress since then and I think we need to ensure Vitest a conforms to the latest configuration requirements.

I've included this on the Roadmap, but one thing I've noted is we may be making extra work for ourselves by having to declare JSDOM in every single test file. It looks like you can now configure this globally:
https://vitest.dev/guide/features.html#mocking

I'm sure there's other issues like this, so my goal is to almost reboot and reinstall Vitest from the ground up. Go test-by-test and ensure there's no extra "fat". This may be a heavy lift, so no worries if you want to leave this for Thomas and I post-a11y update!

@endigo9740
Copy link
Contributor

If you can find a solution for the issues above I'll welcome a follow up PR though.

@endigo9740 endigo9740 closed this Aug 13, 2022
@niktek
Copy link
Contributor Author

niktek commented Aug 15, 2022

Pretty sure this is a case of not doing an npm i or pnpm I after merging in. Tested with both, all tests run fine, no regressions on docs/components

@endigo9740 endigo9740 reopened this Aug 16, 2022
@niktek
Copy link
Contributor Author

niktek commented Aug 16, 2022

This is being rolled up into another PR that upgrades to Svelte Kit 408

@niktek niktek closed this Aug 16, 2022
@niktek niktek deleted the bug/package-alterations branch August 16, 2022 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants