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

Support Vite 6 resolve conditions #163

Merged
merged 2 commits into from
Nov 26, 2024

Conversation

bluwy
Copy link
Contributor

@bluwy bluwy commented Nov 18, 2024

In Vite 6, the top-level resolve.conditions only apply to the client environment, so to apply in all environments generically, we need to use the new configEnvironment hook. https://main.vite.dev/guide/migration.html#default-value-for-resolve-conditions

I adapted this change from sveltejs/vite-plugin-svelte#1020. I also remove the part where it updates the development and browser conditions for Vite 6. I've not tested them with Vite 6, but I feel like it could interfere with the default behaviour.

In practice I don't think a plugin should be modifying that. For development, Vite should decide by itself based on NODE_ENV. For browser, that seems Vitest specific and I'm not sure Vite plugins should be hardcoding Vitest specific logic.

Copy link

changeset-bot bot commented Nov 18, 2024

🦋 Changeset detected

Latest commit: cd9b4fa

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

This PR includes changesets to release 1 package
Name Type
vite-plugin-solid 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

@bluwy bluwy marked this pull request as draft November 21, 2024 04:13
@bluwy
Copy link
Contributor Author

bluwy commented Nov 21, 2024

Marked this as draft for now as I'm noticing some quirks with this approach on the svelte side.

@bluwy bluwy marked this pull request as ready for review November 21, 2024 14:02
@bluwy
Copy link
Contributor Author

bluwy commented Nov 21, 2024

This is ready now, I'd appreciate if I can get some reviews for this as we're releasing Vite 6 soon. The breaking change fix here will also allow Astro w/ Solid to work properly in Astro's next major.

@ryansolid ryansolid merged commit d1bf6b9 into solidjs:main Nov 26, 2024
@ryansolid
Copy link
Member

Thank you

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