-
Notifications
You must be signed in to change notification settings - Fork 925
Refactor preview mode and ensure compatibility with Vite 7 #9647
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
Conversation
🦋 Changeset detectedLatest commit: 48321f6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
@@ -12,6 +12,6 @@ test.runIf(!process.env.CI)( | |||
"should be able to use pg library to send a query", | |||
async () => { | |||
const result = await getJsonResponse("/send-query"); | |||
expect(result!.id).toEqual("1"); | |||
expect(result!.id).toEqual("21"); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 😢
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
wrangler
commit: |
There was a problem hiding this 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.
@@ -12,6 +12,6 @@ test.runIf(!process.env.CI)( | |||
"should be able to use pg library to send a query", | |||
async () => { | |||
const result = await getJsonResponse("/send-query"); | |||
expect(result!.id).toEqual("1"); | |||
expect(result!.id).toEqual("21"); |
There was a problem hiding this comment.
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` |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (resolvedPluginConfig.type === "preview") { | ||
return { appType: "custom" }; | ||
} | ||
|
There was a problem hiding this comment.
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.
Ugh! Conflicts |
3ec5db0
to
4d4162e
Compare
4d4162e
to
48321f6
Compare
…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) ...
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.