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-plugin: update to use the new @cloudflare/unenv-preset #7830

Merged
merged 13 commits into from
Feb 11, 2025

Conversation

petebacondarwin
Copy link
Contributor

@petebacondarwin petebacondarwin commented Jan 20, 2025

Fixes #8054

  • Updates the Vite plugin to use the new Cloudflare unenv preset
  • Updates the Vite plugin to support Vite 6.1 (fixing the issues with the node-compat)
  • Refactors the node-compat stuff to isolate it all in a single Vite plugin

  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because:
  • E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because: not related to Wrangler e2e tests
  • Public documentation
    • TODO (before merge)
    • Cloudflare docs PR(s):
    • Documentation not necessary because: refactoring/bug fix

Copy link

changeset-bot bot commented Jan 20, 2025

🦋 Changeset detected

Latest commit: 6299a92

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

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

Copy link
Contributor

github-actions bot commented Jan 20, 2025

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13264468734/npm-package-wrangler-7830

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7830/npm-package-wrangler-7830

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13264468734/npm-package-wrangler-7830 dev path/to/script.js
Additional artifacts:

cloudflare-workers-bindings-extension:

wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13264468734/npm-package-cloudflare-workers-bindings-extension-7830 -O ./cloudflare-workers-bindings-extension.0.0.0-vc4fc52576.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-vc4fc52576.vsix

create-cloudflare:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13264468734/npm-package-create-cloudflare-7830 --no-auto-update

@cloudflare/kv-asset-handler:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13264468734/npm-package-cloudflare-kv-asset-handler-7830

miniflare:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13264468734/npm-package-miniflare-7830

@cloudflare/pages-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13264468734/npm-package-cloudflare-pages-shared-7830

@cloudflare/unenv-preset:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13264468734/npm-package-cloudflare-unenv-preset-7830

@cloudflare/vite-plugin:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13264468734/npm-package-cloudflare-vite-plugin-7830

@cloudflare/vitest-pool-workers:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13264468734/npm-package-cloudflare-vitest-pool-workers-7830

@cloudflare/workers-editor-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13264468734/npm-package-cloudflare-workers-editor-shared-7830

@cloudflare/workers-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13264468734/npm-package-cloudflare-workers-shared-7830

@cloudflare/workflows-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13264468734/npm-package-cloudflare-workflows-shared-7830

Note that these links will no longer work once the GitHub Actions artifact expires.


[email protected] includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20250129.0
workerd 1.20250204.0 1.20250204.0
workerd --version 1.20250204.0 2025-02-04

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

@petebacondarwin petebacondarwin force-pushed the pbd/update-vite-plugin-to-use-cloudflare-unenv-preset branch 6 times, most recently from 169c800 to a2cd688 Compare February 10, 2025 13:46
@petebacondarwin petebacondarwin marked this pull request as ready for review February 10, 2025 13:50
@petebacondarwin petebacondarwin requested review from a team as code owners February 10, 2025 13:50
@@ -40,9 +40,10 @@
"test:ci": "vitest run"
},
"dependencies": {
"@cloudflare/unenv-preset": "~1.1.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Not sure if this should be "1.1.1" (no ~)? We can relax the range once 2.0 is released
  • Is it expected that the plugin has no (peer) dep on workerd?

Copy link
Contributor

@vicb vicb Feb 10, 2025

Choose a reason for hiding this comment

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

wrangler is in both devDep and peerDep, as is Vite

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wrangler is there twice because the devDep is only for testing (and we use the workspace version); while the peerDep is a range ("wrangler": "^3.101.0") which constrains the minimum version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to pin this dep to 1.1.1 if that helps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the cf unenv preset has its own peer dependency on workerd.

Copy link
Contributor

Choose a reason for hiding this comment

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

Happy to pin this dep to 1.1.1 if that helps

It's more that we do not expect any 1.1.2...

the cf unenv preset has its own peer dependency on workerd.

it's a peer dep so it should be installed somewhere...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It'll be installed as part of Wrangler

Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

I only took a quick look.

How urgent is this? If this can be put on hold until we release the v2 of the preset, that would be ideal

@petebacondarwin
Copy link
Contributor Author

I only took a quick look.

How urgent is this? If this can be put on hold until we release the v2 of the preset, that would be ideal

The Vite plugin is currently broken with Vite 6.1 and this solves that. If we want to hold off on landing this then I would have to put up a fix for the old unenv usage.

Is there a reason to hold this back? This Vite plugin is, itself, in beta, so it feels like a good place to try out new stuff like the Cloudflare unenv update.

@petebacondarwin petebacondarwin force-pushed the pbd/update-vite-plugin-to-use-cloudflare-unenv-preset branch from 85717f6 to 5e2abeb Compare February 10, 2025 14:15
@vicb
Copy link
Contributor

vicb commented Feb 10, 2025

Is there a reason to hold this back? This Vite plugin is, itself, in beta, so it feels like a good place to try out new stuff like the Cloudflare unenv update.

I'm asking mostly because I don't have time to do a deep review.
But if everything is still in beta, there should be no problem.

There might be changes to do after #7915 is merged / 2.0 is released

Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

No time for a deep review right now.
But @petebacondarwin mentioned it fixes issues and the plugin is still in beta.
I can look at it more closely after v2.0 of the preset is released

Copy link
Contributor

@jamesopstad jamesopstad 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, as far as I understand the new preset. I would prefer the plugins to be defined in the same file though so that they have access to the cloudflare function scope.

packages/vite-plugin-cloudflare/src/node-js-compat.ts Outdated Show resolved Hide resolved
packages/vite-plugin-cloudflare/src/node-js-compat.ts Outdated Show resolved Hide resolved
packages/vite-plugin-cloudflare/src/node-js-compat.ts Outdated Show resolved Hide resolved
packages/vite-plugin-cloudflare/src/node-js-compat.ts Outdated Show resolved Hide resolved
packages/vite-plugin-cloudflare/src/node-js-compat.ts Outdated Show resolved Hide resolved
packages/vite-plugin-cloudflare/src/node-js-compat.ts Outdated Show resolved Hide resolved
@jamesopstad jamesopstad added the vite-plugin Relating to the `@cloudflare/vite-plugin` package label Feb 10, 2025
@petebacondarwin petebacondarwin force-pushed the pbd/update-vite-plugin-to-use-cloudflare-unenv-preset branch from 5e2abeb to 9291388 Compare February 10, 2025 18:56
Copy link
Contributor

@jamesopstad jamesopstad 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! I think there's still a bit of an open question about what's correct for preview mode but happy for you to make the decision on this.

packages/vite-plugin-cloudflare/src/index.ts Outdated Show resolved Hide resolved
packages/vite-plugin-cloudflare/src/index.ts Show resolved Hide resolved
@petebacondarwin petebacondarwin force-pushed the pbd/update-vite-plugin-to-use-cloudflare-unenv-preset branch from e931930 to 6299a92 Compare February 11, 2025 13:34
@petebacondarwin petebacondarwin force-pushed the pbd/update-vite-plugin-to-use-cloudflare-unenv-preset branch from 6299a92 to b46a906 Compare February 11, 2025 13:51
@penalosa penalosa merged commit 99ba292 into main Feb 11, 2025
18 checks passed
@penalosa penalosa deleted the pbd/update-vite-plugin-to-use-cloudflare-unenv-preset branch February 11, 2025 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vite-plugin Relating to the `@cloudflare/vite-plugin` package
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

🐛 BUG: @cloudflare/vite-plugin Vite 6.1.0 vite dev errors when nodejs_compat is enabled
4 participants