Skip to content

Conversation

@0xbad0c0d3
Copy link
Contributor

@0xbad0c0d3 0xbad0c0d3 commented Apr 24, 2025

This update introduces a playground that exercises support for the run_worker_first flag when running vite preview.
Currently in vite dev static (non-HTML) assets are still handled by Vite before getting to the Worker, so there are failing use cases there.


  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because:
  • Wrangler / Vite E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because: e2e tests for Vite don't cover this code and it is tested enough in the fixture.
  • Public documentation
  • Wrangler V3 Backport
    • TODO (before merge)
    • Wrangler PR:
    • Not necessary because: vite-plugin is not backported

@0xbad0c0d3 0xbad0c0d3 requested review from a team as code owners April 24, 2025 11:22
@changeset-bot
Copy link

changeset-bot bot commented Apr 24, 2025

⚠️ No Changeset found

Latest commit: ab82f46

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@petebacondarwin
Copy link
Contributor

Hey thanks for submitting this change @0xbad0c0d3. Unfortunately I think it is a bit more involved to make this work fully.
At the moment Vite will handle all non-HTML assets before it ever gets to the Worker, so whatever the configuration of Miniflare, the Worker will not get to intercept such requests.

I have a draft Worker that solves this problem more fully - #8494 - but due to the complexity it adds to the implementation, we want land it behind a flag. I hope to have that ready (at least the run_worker_first bits) by the end of this week.

@0xbad0c0d3
Copy link
Contributor Author

@petebacondarwin sorry. I didn't get your words:

At the moment Vite will handle all non-HTML assets before it ever gets to the Worker

Isn't it must be so? How other way the worker will know about request? If vite-server is the entry-point of the request...
Sorry for the, probably, dummy question ))

@petebacondarwin
Copy link
Contributor

The Vite dev server is basically like an express server that has middleware. The Worker gets triggered by a middleware at the very end of the chain after all the other middlewares have had a chance to deal with the request. We have turned off most of the built in middleware so that we can handle HTML requests. But there is still middleware that will identify requests that match assets (either static or generated client side code) and will just respond with that content instead of passing it through to the other middlewares, such as the one that triggers the Worker.

@petebacondarwin
Copy link
Contributor

Actually, I was just going through my bigger PR and I realised that just adding this to miniflare options is enough to get it working in vite preview mode (but not vite dev mode). I have a playground fixture in my PR that tests this so I will push a commit here and I think we can land this PR, with a few tweaks, to support in preview mode at least.

Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

I don't think we can solve the vite dev mode issue so simply. I propose we change this PR to only add support for vite preview, which is quite straightforward, and then we can look into vite dev separately.

@0xbad0c0d3 0xbad0c0d3 force-pushed the vite-plugin-cloudflare-worker-execution-order branch from 79e6192 to bce43f0 Compare April 25, 2025 09:03
@0xbad0c0d3
Copy link
Contributor Author

I don't think we can solve the vite dev mode issue so simply. I propose we change this PR to only add support for vite preview, which is quite straightforward, and then we can look into vite dev separately.

yes, you are right, I've just got what exactly are you talking about! Rolled back the changes I've made. Will also think about it.

@petebacondarwin petebacondarwin self-assigned this Apr 25, 2025
@petebacondarwin
Copy link
Contributor

Pushing a commit now...

@petebacondarwin petebacondarwin force-pushed the vite-plugin-cloudflare-worker-execution-order branch from bce43f0 to 11e434c Compare April 25, 2025 09:13
@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Apr 25, 2025
@petebacondarwin
Copy link
Contributor

This tweaks the way we update the miniflare config to be a bit simpler, and adds back in the fixture but only tests it in vite preview mode. I also tweaked the changeset to be more user facing and less implementation oriented.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 25, 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/14664755364/npm-package-wrangler-9044
Prereleases for other packages:

cloudflare-workers-bindings-extension:

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

create-cloudflare:

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

@cloudflare/kv-asset-handler:

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

miniflare:

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

@cloudflare/pages-shared:

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

@cloudflare/unenv-preset:

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

@cloudflare/vite-plugin:

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

@cloudflare/vitest-pool-workers:

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

@cloudflare/workers-editor-shared:

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

@cloudflare/workers-shared:

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

@cloudflare/workflows-shared:

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

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

cod1k and others added 7 commits April 25, 2025 10:53
This update introduces support for the `run_worker_first` flag when configuring Miniflare options. A new test suite ensures proper handling of this flag, including scenarios where it is undefined. Additionally, the `miniflare-options` logic was updated to pass the flag to the worker configuration when specified.
@petebacondarwin petebacondarwin force-pushed the vite-plugin-cloudflare-worker-execution-order branch from e91bf39 to 4a88738 Compare April 25, 2025 09:53
@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Apr 25, 2025
@petebacondarwin petebacondarwin changed the title Add support for run_worker_first flag in Miniflare options Vite-plugin: Add support for run_worker_first flag in vite preview mode Apr 25, 2025
@github-project-automation github-project-automation bot moved this from Approved to In Review in workers-sdk Apr 25, 2025
@petebacondarwin petebacondarwin changed the title Vite-plugin: Add support for run_worker_first flag in vite preview mode Vite-plugin: Add playground tests for run_worker_first support in vite preview mode Apr 25, 2025
@petebacondarwin petebacondarwin dismissed jamesopstad’s stale review April 25, 2025 13:15

The concern was addressed. This PR is now only adding tests.

@petebacondarwin petebacondarwin merged commit d34ef3d into cloudflare:main Apr 25, 2025
27 of 34 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in workers-sdk Apr 25, 2025
@workers-devprod workers-devprod added the contribution [Holopin] Recognizes an open-source contribution, big or small label Apr 25, 2025
@holopin-bot
Copy link

holopin-bot bot commented Apr 25, 2025

Congratulations @0xbad0c0d3, the maintainer of this repository has issued you a holobyte! Here it is: https://holopin.io/holobyte/cm9wuj1me50410cjm5ebakxm9

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution [Holopin] Recognizes an open-source contribution, big or small no-changeset-required

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants