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

Vite 5 support #127

Merged
merged 4 commits into from
Dec 7, 2023
Merged

Vite 5 support #127

merged 4 commits into from
Dec 7, 2023

Conversation

birkskyum
Copy link
Contributor

@birkskyum birkskyum commented Nov 18, 2023

Closes #126

I updated the examples to have separate examples for Vite 3, 4 and 5, and checked they all work.

I added vite 5 to the peer deps:

"peerDependencies": {
"solid-js": "^1.7.2",
"vite": "^3.0.0 || ^4.0.0 || ^5.0.0"
},

@Grabber
Copy link

Grabber commented Nov 20, 2023

Highest priority!

@carere
Copy link

carere commented Nov 24, 2023

For info, with vite 5, the solid-start-deno adapter does not work when building the project.
Getting an error with vite:define

@birkskyum
Copy link
Contributor Author

birkskyum commented Nov 24, 2023

solid-start is kind of a next step after this. I also stumbled upon some issues with adapters solidjs/solid-start#1124 , but figured it's probably more feasible to await the new Nitro/Vinxi version, which replace i.e. solid-start-deno, and then have a look at it.

@nilshelmig
Copy link

@ryansolid @birkskyum What's missing to get this merged?

@birkskyum
Copy link
Contributor Author

I'm not currently aware of anything missing.

@asterikx
Copy link

asterikx commented Dec 1, 2023

I'm also getting errors from vite:define using Vite 5 and solid-start-cloudflare-pages@0.3.10:

user@MBP my-project % pnpm build

> my-solid-app@0.1.0 build /Users/user/my-project
> solid-start build

 solid-start build 
 version  0.3.10
(node:93293) ExperimentalWarning: The Ed25519 Web Crypto API algorithm is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
 adapter  cloudflare-pages

solid-start building client...
vite v5.0.4 building for production...
✓ 4 modules transformed.
[vite:define] Transform failed with 1 error:
error: Invalid define value (must be an entity name or valid JSON syntax): (() => {})
file: /Users/user/my-project/node_modules/.pnpm/solid-start@0.3.10_@solidjs+meta@0.29.1_@solidjs+router@0.9.1_solid-js@1.8.6_solid-start-clou_ykgf2egikbvfaqjxxzhhreqmsa/node_modules/solid-start/entry-client/mount.tsx
🐼 error [❌] Transform failed with 1 error:
error: Invalid define value (must be an entity name or valid JSON syntax): (() => {})

@birkskyum
Copy link
Contributor Author

birkskyum commented Dec 1, 2023

I don't think this will fix the solid-start-cloudflare-pages adapter. It'll be replace with this adapter soon, so it might help.

@nilshelmig
Copy link

I'm also getting errors from vite:define using Vite 5 and solid-start-cloudflare-pages@0.3.10:

user@MBP my-project % pnpm build

> my-solid-app@0.1.0 build /Users/user/my-project
> solid-start build

 solid-start build 
 version  0.3.10
(node:93293) ExperimentalWarning: The Ed25519 Web Crypto API algorithm is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
 adapter  cloudflare-pages

solid-start building client...
vite v5.0.4 building for production...
✓ 4 modules transformed.
[vite:define] Transform failed with 1 error:
error: Invalid define value (must be an entity name or valid JSON syntax): (() => {})
file: /Users/user/my-project/node_modules/.pnpm/solid-start@0.3.10_@solidjs+meta@0.29.1_@solidjs+router@0.9.1_solid-js@1.8.6_solid-start-clou_ykgf2egikbvfaqjxxzhhreqmsa/node_modules/solid-start/entry-client/mount.tsx
🐼 error [❌] Transform failed with 1 error:
error: Invalid define value (must be an entity name or valid JSON syntax): (() => {})

Is this related to vite-plugin-solid? For me it seems, this relates to solid-start-cloudflare-pages. If so, consider reporting the bug there 🙂

@CreativeTechGuy
Copy link

Can/Should this PR also fix the deprecated APIs that were being used? I know we have been getting spammed with warnings when running tests as a result of registerNodeLoader. This was removed in vitest 1.0 here and since Vite 5 and Vitest 1 are tightly coupled, it'd make sense to fix that here.

@birkskyum
Copy link
Contributor Author

birkskyum commented Dec 6, 2023

How do I do that? Is it a matter of replacing registerNodeLoader with optimizer as indicated here:

deps: { registerNodeLoader: true },

replaced with:

deps: { 
  optimizer: {
    ssr: {enabled: true},
    web: {enabled: true}
  }
},

Or should deps: { registerNodeLoader: true }, simply be removed, since the enabled: true is the default value anyway?

@CreativeTechGuy
Copy link

@birkskyum I'm not sure. It might work just by being removed. The docs you found seem to point to just removing it.

@birkskyum
Copy link
Contributor Author

Think so too - guess there's a reason for this issue being opened:

I have removed it.

@ryansolid
Copy link
Member

Thanks everyone. Sorry for taking my time here. Often with new versions we hit some fun issues..So was just letting it breathe.

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.

Upgrade compatibility with Vite 5
7 participants