Skip to content

Refactor preview mode and ensure compatibility with Vite 7#9647

Merged
CarmenPopoviciu merged 14 commits intomainfrom
james/refactor-preview
Jun 18, 2025
Merged

Refactor preview mode and ensure compatibility with Vite 7#9647
CarmenPopoviciu merged 14 commits intomainfrom
james/refactor-preview

Conversation

@jamesopstad
Copy link
Contributor

@jamesopstad jamesopstad commented Jun 18, 2025

Fixes #000.

In Vite 7, the applyToEnvironment hook is called in preview mode. This is now accounted for to ensure compatibility. I have refactored the resolved plugin config so that it now returns a config for preview mode.

Additionally, the root property of the module runner options has been removed as it is not needed and is dropped in Vite 7.


  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because: already covered by tests
  • Wrangler / Vite E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because:
  • Public documentation
    • TODO (before merge)
    • Cloudflare docs PR(s):
    • Documentation not necessary because: internal change
  • Wrangler V3 Backport
    • TODO (before merge)
    • Wrangler PR:
    • Not necessary because: not a Wrangler change

@jamesopstad jamesopstad requested a review from a team June 18, 2025 11:14
@jamesopstad jamesopstad requested a review from a team as a code owner June 18, 2025 11:14
@jamesopstad jamesopstad added e2e Run wrangler + vite-plugin e2e tests on a PR vite-plugin Relating to the `@cloudflare/vite-plugin` package labels Jun 18, 2025
@changeset-bot
Copy link

changeset-bot bot commented Jun 18, 2025

🦋 Changeset detected

Latest commit: 48321f6

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

async () => {
const result = await getJsonResponse("/send-query");
expect(result!.id).toEqual("1");
expect(result!.id).toEqual("21");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test was consistently failing with this result so I assume the data in the database has changed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Really we should make a fake pg instance somewhere that we host ourselves for this (maybe even locally!) to avoid this sort of problem. This test also often fails because we get rate limited 😢

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jun 18, 2025

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@9647

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@9647

miniflare

npm i https://pkg.pr.new/miniflare@9647

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@9647

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@9647

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@9647

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@9647

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@9647

wrangler

npm i https://pkg.pr.new/wrangler@9647

commit: 48321f6

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.

Nice bit of refactor. Some non-blocking suggestions about the types.

async () => {
const result = await getJsonResponse("/send-query");
expect(result!.id).toEqual("1");
expect(result!.id).toEqual("21");
Copy link
Contributor

Choose a reason for hiding this comment

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

Really we should make a fake pg instance somewhere that we host ourselves for this (maybe even locally!) to avoid this sort of problem. This test also often fails because we get rate limited 😢

* Gets the content of the `.dev.vars` target file
*
* Note: This resolves the .dev.vars file path following the same logic
* as `loadDotEnv` in `/packages/wrangler/src/config/index.ts`
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update the comment in the referenced file

* Note: The `getDotDevDotVarsContent` function in the `packages/vite-plugin-cloudflare/src/index.ts` file
to point back to this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +105 to +108
if (resolvedPluginConfig.type === "preview") {
return { appType: "custom" };
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call to move this down.

@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Jun 18, 2025
@petebacondarwin
Copy link
Contributor

Ugh! Conflicts

@jamesopstad jamesopstad force-pushed the james/refactor-preview branch from 3ec5db0 to 4d4162e Compare June 18, 2025 15:00
@jamesopstad jamesopstad requested a review from a team as a code owner June 18, 2025 15:00
@jamesopstad jamesopstad force-pushed the james/refactor-preview branch from 4d4162e to 48321f6 Compare June 18, 2025 15:15
@CarmenPopoviciu CarmenPopoviciu merged commit 6c6afbd into main Jun 18, 2025
19 checks passed
@CarmenPopoviciu CarmenPopoviciu deleted the james/refactor-preview branch June 18, 2025 17:08
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk Jun 18, 2025
jseba added a commit to jseba/workers-sdk that referenced this pull request Jun 18, 2025
…seba/containers_scope

* 'main' of ssh://github.com/cloudflare/workers-sdk: (31 commits)
  Refactor preview mode and ensure compatibility with Vite 7 (cloudflare#9647)
  Block requests vulnerable to opennext vulnerability (cloudflare#9635)
  Add test for cloudchamber buildAndMaybePush (cloudflare#9638)
  chore: remove redundant binding guide superseded by internal docs (cloudflare#9648)
  add changeset to trigger release of workers/pages projects (cloudflare#9649)
  Add @handler to Python templates. (cloudflare#9305)
  Migrate from unbuild to obuild (cloudflare#9243)
  Version Packages (cloudflare#9650)
  fix changeset (cloudflare#9651)
  containers: Default scheduling policy should be the default (cloudflare#9621)
  Rename Mixed Mode to remote proxy/remote bindings depending on context (cloudflare#9586)
  Version Packages (cloudflare#9632)
  Correctly mock out getDockerImageDigest for testing buildAndMaybePush (cloudflare#9636)
  [C3] Bump create-remix from 2.16.6 to 2.16.8 in /packages/create-cloudflare/src/frameworks (cloudflare#9525)
  Remove "Cloudchamber" from user facing error messages (cloudflare#9628)
  sync local containers with latest workerd (cloudflare#9576)
  Bump the workerd-and-workers-types group with 2 updates (cloudflare#9591)
  [C3] Bump gatsby from 5.14.3 to 5.14.4 in /packages/create-cloudflare/src/frameworks (cloudflare#9524)
  [C3] Bump create-react-router from 7.6.1 to 7.6.2 in /packages/create-cloudflare/src/frameworks (cloudflare#9526)
  [C3] Bump create-docusaurus from 3.8.0 to 3.8.1 in /packages/create-cloudflare/src/frameworks (cloudflare#9527)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e2e Run wrangler + vite-plugin e2e tests on a PR vite-plugin Relating to the `@cloudflare/vite-plugin` package

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants