[create-cloudflare] Preserve existing Nuxt modules when adding Cloudflare configuration#14534
Conversation
🦋 Changeset detectedLatest commit: eba23b6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
|
✅ All changesets look good |
|
The tests are well-structured and cover the main cases. Let me check the changeset descriptions against the repo rules — the changeset content mentions internal package Let me verify one potential concern — the changeset bot warning mentioned in the PR comment referenced The changesets look correct. Now let me assess the overall correctness. The core logic is sound. Let me consider one real edge case: de-duplication uses One remaining concern worth noting: Let me double check there isn't a subtle issue with the The change is focused, correct, well-tested, and the changesets and e2e updates are appropriate. The logic properly:
I found no actionable logic bugs, security issues, backward-compat violations, or incorrect API behavior. The de-dup-only-for-strings limitation is acceptable given actual usage and not worth flagging as it would be a nitpick on hypothetical scenarios. LGTM |
@cloudflare/autoconfig
@cloudflare/config
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
|
Codeowners approval required for this PR:
Show detailed file reviewers |
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
…dflare configuration The shared codemod's `mergeObjectProperties` overwrote array properties instead of merging them. Scaffolding (C3) or configuring (`@cloudflare/autoconfig`) a Nuxt app whose `nuxt.config.ts` already declared a `modules` array (e.g. the `ui` starter, which registers `@nuxt/ui` and `@nuxt/eslint`) therefore dropped those modules when adding `nitro-cloudflare-dev`, breaking the build. Array properties are now concatenated onto the existing array, de-duplicating string entries, so existing modules are preserved and the Cloudflare module is appended. The Nuxt e2e tests now use the `ui` template (which ships a `modules` array) so this regression is covered.
The nuxi ui template pins packageManager pnpm@11.9.0. Under pnpm 10 the cross-version self-provisioning recurses and fails with ENAMETOOLONG (nested .pnpm/.tools/pnpm/11.9.0_tmp_* paths), so restrict the Nuxt framework e2e tests to pnpm 11+.
… coverage The Nuxt tests use the `ui` template (to exercise the modules-merge fix), gated to pnpm 11+ because `ui` pins packageManager pnpm@11.9.0 and pnpm 10 self-provisions into an ENAMETOOLONG recursion. That left Nuxt with no pnpm 10 coverage. Add `nuxt:pages:minimal` / `nuxt:workers:minimal` variants (default template, gated to pnpm <11) so the pre-pnpm-11 install path stays tested, and disambiguate duplicate vitest titles via a third `:`-segment variant label. npm is skipped on the minimal variants since the ui variant already covers npm.
- Add `unsupportedPms: ["yarn"]` to the experimental nuxt:workers (ui) entry to match its non-experimental counterpart (nitro/youch needs Node 20+; yarn runs on Node 18). - Add `unsupportedPms: ["yarn", "npm"]` to the experimental nuxt:workers:minimal entry for consistency with the non-experimental minimal variants. - Broaden the colon-form test filter from exact-match to a "name:platform" prefix match so filtering by e.g. `nuxt:pages` also selects variant tests like `nuxt:pages:minimal`.
d46a60e to
db2f5f2
Compare
Fixes #14513
C3 and
@cloudflare/autoconfigoverwrote a Nuxt project's existingmodulesarray innuxt.config.tswhen addingnitro-cloudflare-dev. For starters that ship amodulesarray — such asnuxt/starter@ui, which registers@nuxt/uiand@nuxt/eslint— this dropped those modules, so@import "@nuxt/ui"inassets/css/main.cssno longer resolved and the build failed.The root cause is in the shared
@cloudflare/codemodmergeObjectPropertieshelper: it deep-merged object properties but overwrote array properties. It now concatenates new elements onto the existing array, de-duplicating string entries, so existing modules are preserved and the Cloudflare module is appended.What changed
@cloudflare/codemod:mergeObjectPropertiesnow merges (concatenates + de-dupes) array properties instead of overwriting them, with unit tests covering concat, de-dupe, and mixed array+object merges.create-cloudflareand@cloudflare/autoconfigboth bundle the codemod, so both pick up the fix (a patch changeset is added for each).uitemplate (which ships amodulesarray) so this regression is covered — the previousminimaltemplate had nomodulesarray and could not reproduce the bug.Before / after
C3's exact transform run against the real
uistarternuxt.config.ts:modules: ["nitro-cloudflare-dev"]—@nuxt/uiand@nuxt/eslintdroppedmodules: ['@nuxt/ui', '@nuxt/eslint', "nitro-cloudflare-dev"]