Repository navigation
[vitest-plugin] Support cloudflare.config.ts with the new config option - #16424
edmundhung wants to merge 9 commits into
Conversation
🦋 Changeset detectedLatest commit: 6400e04 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 |
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/codemods
@cloudflare/config
@cloudflare/containers-shared
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/runtime-types
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-plugin
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
|
LGTM! |
b8c4eca to
78e99da
Compare
edmundhung
changed the title
78e99da to
b56a0dd
Compare
b56a0dd to
25ebf5f
Compare
….newConfig `experimental.newConfig` converted `cloudflare.config.ts` to a Wrangler config and ran it through Wrangler's `unstable_getMiniflareWorkerOptions()`. The parsed config is already in the shape Miniflare's native `WorkerOptions.config` takes, so apply it to the runner Worker directly and stop importing `wrangler` on this path. Local secrets and remote bindings use copies of `@cloudflare/vite-plugin`'s glue, and remote bindings authenticate with cf credentials. Wrangler configuration files are handled exactly as before. This also fixes Durable Object bindings to the config's own Worker, which the conversion turned into bindings to another Worker. Refs DEVX-2645
Load `cloudflare.config.ts` through a stable `config` option: `true` for the project root, or a path to the file. As with `wrangler`, the file is only loaded when the option is set, and setting both is an error. `experimental.newConfig` is removed; passing it now fails with a pointer to `config` rather than being silently ignored. If the file fails to load and its name mentions `wrangler`, the error suggests the `wrangler` option instead. Refs DEVX-2645
`readD1Migrations()` in `@cloudflare/vitest-plugin` imported all of Wrangler just to split migration files into statements. Move `splitSqlQuery()` and its trimmer into `@cloudflare/workers-utils`, with their tests, and import them from there in Wrangler and the Vitest plugin. Wrangler's `unstable_splitSqlQuery` export is unchanged. Refs DEVX-2645
25ebf5f to
1b451aa
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
|
💬 Approved with comments — No correctness, security or performance problems found. The only note is an optional README update for the WebAssembly fixture, which should mention that default module rules now also apply with the new Findings: 1 info The decision is advisory: this comment never approves, blocks or requests changes on the pull request. 🔧 Fix with your agentPaste the following into your coding agent to investigate and fix these issues. 📝 Documentation: 💬 1 infoThe WebAssembly fixture README is out of date about default module rules. Recommendation: 💬 Optional Nits
View full review
✅ All other checks passed🚀 Release: ✅ LGTMRisk: Low. The changeset covers the new 🔒 Security: ✅ LGTM
📖 AGENTS.md: ✅ LGTMThe 📋 Code Quality: ✅ LGTMNo code quality issues found. ⚡ Performance: ✅ LGTM
Reviewed commit: |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| options.moduleRules = [ | ||
| ...DEFAULT_MODULE_RULES, | ||
| ...(options.moduleRules ?? []), | ||
| ]; |
There was a problem hiding this comment.
🟡 Custom module rules are shadowed by defaults
With config enabled, DEFAULT_MODULE_RULES precedes user rules. compileModuleRules finalizes each default type first, silently discarding custom rules of that type.
Learn more
The pool uses compileModuleRules when intercepting test module imports. Unless a rule enables fallthrough, the compiler ignores every later rule of the same type. The three defaults here all omit fallthrough, so any user-defined Text, Data, or CompiledWasm rule is discarded. Wrangler orders user rules before defaults in unstable_getMiniflareWorkerOptions.
Example: Setting miniflare.modulesRules: [{ type: "Text", include: ["**/*.md"] }] alongside config: true does not activate text loading for .md files; the default Text rule finalizes that type first.
Recommended fix: Place user rules before default rules, preserving their precedence as in the Wrangler path. Check that fallthrough behavior remains correct for user rules extending a default type.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for (const [key, binding] of Object.entries(cloudflareConfig?.env ?? {})) { | ||
| if (binding.type === "durable-object") { | ||
| result.set(key, { | ||
| className: binding.exportName, | ||
| // A binding to the config's own Worker targets the (renamed) runner | ||
| scriptName: | ||
| binding.worker === cloudflareConfig?.worker.name | ||
| ? undefined | ||
| : binding.worker, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🟡 Overridden binding breaks Durable Object helpers
When miniflare.bindings overrides a config Durable Object with text, getDurableObjectDesignators still registers it. getSameIsolateNamespaces then rejects even unrelated valid Durable Object stubs.
Learn more
The runner merges miniflare bindings over config bindings in applyCloudflareConfig. The new designator loop instead records the config binding without consulting those overrides. getSameIsolateNamespaces examines every recorded binding and asserts that its runtime value is a Durable Object namespace. One stale map entry causes any subsequent same-isolate helper call to fail.
Example: Config declares OLD and COUNTER as Durable Objects. Pool options set miniflare.bindings.OLD = "disabled". Calling runInDurableObject on a COUNTER stub fails while examining OLD, which is now a string.
Recommended fix: Build the designator map from the effective merged bindings, or skip config entries overridden by any miniflare binding type. Keep precedence consistent with applyCloudflareConfig.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // A different hook restarts the session, so reuse the session's own hook | ||
| // while the account is unchanged | ||
| const authKey = JSON.stringify({ accountId: settings.accountId }); | ||
| const preExistingRemoteProxySessionData = | ||
| remoteProxySessionsDataMap.get(configPath); | ||
| const remoteProxySessionData = options.remoteBindings | ||
| ? await maybeStartOrUpdateRemoteProxySession( | ||
| { | ||
| name: worker.name, | ||
| bindings: cloudflareConfig.remoteBindings, | ||
| complianceRegion: | ||
| settings.complianceRegion === "fedramp-high" | ||
| ? "fedramp_high" | ||
| : settings.complianceRegion, | ||
| }, | ||
| preExistingRemoteProxySessionData ?? null, | ||
| (preExistingRemoteProxySessionData?.authKey === authKey | ||
| ? preExistingRemoteProxySessionData.auth | ||
| : undefined) ?? |
There was a problem hiding this comment.
🔍 Profile changes and cached sessions
authKey tracks the configured account, while getRemoteBindingsAuth resolves credentials lazily from a cached profile. Check whether changing credentials during watch mode needs a session restart.
Was this helpful? React with 👍 or 👎 to provide feedback.
… keep internal types unpublished
Fixes DEVX-2645.
Add official
cloudflare.config.tssupport to@cloudflare/vitest-pluginthrough a newconfigoption, and remove the experimentalexperimental.newConfig.Depends on #16428, which should merge first:
src/pool/auth.tsmirrors thegetRemoteBindingsAuth()it adds to@cloudflare/vite-plugin.Please review the full diff rather than commit by commit: follow-up commits added and then removed a native
miniflaresurface, and the repository squash-merges. The main changes:cloudflare.config.tsnatively underexperimental.newConfig.WorkerOptions.configshape. It is no longer converted to a Wrangler config and run throughunstable_getMiniflareWorkerOptions(), and this path no longer importswrangler.@cloudflare/vite-pluginv2's glue, each marked to be kept in sync.cf auth loginorCLOUDFLARE_API_TOKEN), and messages namecf. The session keeps its auth hook while the account is unchanged, so re-parsing a project doesn't restart it.env,exportsand triggers are applied to the runner. Tail consumers aren't supported yet and are dropped with a warning, and so is the assets binding, as there's no assets directory.experimental.newConfigwith theconfigoption.true,falseor a path to the file.wrangler, the file is only loaded when the option is set, and setting both is an error. With neither set, behaviour is unchanged.experimental.newConfigfails with a pointer toconfig.wrangler, the error suggests thewrangleroption.@cloudflare/workers-utils. This meansreadD1Migrations()no longer loads Wrangler. It's an internal refactor, independent of the first two, and can be dropped from this PR on its own.miniflarewithconfigconfigcan't be combined withminiflareoptions, so the Worker is configured entirely bycloudflare.config.ts. A stacked follow-up will add options for test-time changes and auxiliary Workers, modelled on@cloudflare/vite-pluginv2.Secrets on the
configpath.dev.varsand.envfollow the same rules as@cloudflare/vite-pluginand thecfCLI, rather than Wrangler's:envDir(the project root by default);{ type: "secret" }are filled;.dev.varsdoesn't overridetextorjsonbindings;.dev.vars.<mode>and.env.<mode>files are read (Vitest's mode defaults totest).One intentional difference from the Vite plugin, kept from the Wrangler path:
CLOUDFLARE_*tool settings such asCLOUDFLARE_API_TOKENare read from the environment, not from.env. The Vite plugin copies them from.envintoprocess.env, but Vitest runs every project in one process, so that copy would leak one project's values into the others. Supporting them per project is left to a follow-up.CLOUDFLARE_HYPERDRIVE_LOCAL_CONNECTION_STRING_<NAME>fills a binding rather than configuring the tool, so like secrets it's read from each project's.env, as in the Vite plugin.Measurements
A full
vitest runof a minimalcloudflare.config.tsproject goes from a median of 1.11 s (experimental.newConfigonmain) to 0.85 s (n=8, local), mainly becausewrangleris no longer imported.