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

Fix test config #200

Merged
merged 5 commits into from
Feb 27, 2025
Merged

Fix test config #200

merged 5 commits into from
Feb 27, 2025

Conversation

pedro00dk
Copy link
Contributor

Copying my text from discord:

Hi,
I updated to the latest version of vite-plugin-solid (2.11.4) and I believe it is broken.
I'm using Vitest 3 (alrady dealing with the issue from the previous version, not being able to use instances).
After updating to the new version I'm having a different issue, every time I run vitest it asks me to install jsdom.
Even though I have a test.environment = 'node' in my vite configuration.

I had a look at one of the patches and I think I found the issue.
https://github.com/solidjs/vite-plugin-solid/pull/183/files#diff-a2a171449d862fe29692ce031981047d7ab755ae7f84c707aef80701b3ea0c80R222
In the previous version, the code checked for test.environment being not set, where test is vite's userConfig.test
In the new version it is checking for userTest.environment not being set, however, userTest is userConfig, not userConfig.test.
The code is checking the wrong object, same in line 218 for setupFiles.
I'm a bit tired to write an issue right now, too late...


Changes:

  • Fix the plugin to use the correct test object.
  • Updated example/vite-6 to use vitest browser mode and vitest Locator and expect APIs.

Copy link

changeset-bot bot commented Feb 27, 2025

🦋 Changeset detected

Latest commit: 31d8de3

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

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

Copy link

pkg-pr-new bot commented Feb 27, 2025

Open in Stackblitz

npm i https://pkg.pr.new/vite-plugin-solid@200

commit: 31d8de3

@birkskyum
Copy link
Member

birkskyum commented Feb 27, 2025

@pedro00dk thanks! can you add a changeset pnpm changeset?

@birkskyum
Copy link
Member

birkskyum commented Feb 27, 2025

can you add a CI flow that run the vitest tests?

@pedro00dk
Copy link
Contributor Author

Hi, I can do that.
Just not atm. Will do in 2-3 hours.

@pedro00dk
Copy link
Contributor Author

pedro00dk commented Feb 27, 2025

I mostly use gitlab, I don't know much about github CI though.
Not sure how to test them, and it seems somebody has to approve it before it can run...

Copy link
Member

@birkskyum birkskyum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@birkskyum birkskyum merged commit 14da18d into solidjs:main Feb 27, 2025
4 checks passed
@pedro00dk
Copy link
Contributor Author

CI first try lul

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