Skip to content

Generate runtime types when using the new config in vite dev and wrangler dev - #14652

Merged
jamesopstad merged 4 commits into
mainfrom
james/new-config-runtime-types
Jul 13, 2026
Merged

jamesopstad merged 4 commits into
mainfrom
james/new-config-runtime-types

Conversation

@jamesopstad

Copy link
Copy Markdown
Contributor

  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: experimental feature

A picture of a cute animal (not mandatory, but encouraged)

@changeset-bot

changeset-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: a1d707d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@cloudflare/vite-plugin Minor
wrangler Minor
@cloudflare/vitest-pool-workers 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

@github-actions

Copy link
Copy Markdown
Contributor

✅ All changesets look good

@ask-bonk

ask-bonk Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

All correct. Let me do a final review of the assembled content ordering in new-config.ts to check for a subtle bug: the config content from generateTypes ends with a trailing blank line (\n\n). Then runtime is appended. Let me verify this doesn't create a double blank that breaks the RUNTIME_HEADER line detection — but the header detection uses startsWith(prefix) on split lines, so leading blank lines are fine.

I've completed a thorough review. Let me summarize my findings:

Summary of the PR:

  • Refactors generateRuntimeTypes to accept existingContent instead of outFile (caller reads the file once)
  • Changes @cloudflare/config generateTypes to emit a global script (inline import(...) types, no top-level imports/declare global) so appended ambient runtime types stay global
  • Adds includeRuntime option to both vite plugin and wrangler experimental new-config paths, appending runtime types to worker-configuration.d.ts during dev
  • Makes vite plugin type generation dev-only (command === "serve")

Assessment:

  • The core design decision (global-script form to allow appended ambient globals) is sound and well-tested
  • The single-read refactor is clean, well-documented, and removes redundant file reads
  • Field mapping (compatibilityDate/compatibilityFlags → snake_case) is correct against the schema
  • The runtime-types cache logic correctly scans the full file for header/marker
  • workerd correctly added as external dependency; @cloudflare/runtime-types as bundled devDependency, consistent with @cloudflare/config
  • Tests are comprehensive: cover defaults, includeRuntime: false, build-vs-serve, global-script form
  • Changesets are correct (minor for both, documenting the dev-only behavior change)

The only minor behavioral change I noticed — wrangler's index.ts now swallows all file-read errors (not just ENOENT) — is defensive and non-user-facing for a cache read, so it's not actionable.

No logic bugs, security issues, backward-compat violations, or incorrect API behavior found.

LGTM

github run

@pkg-pr-new

pkg-pr-new Bot commented Jul 10, 2026

Copy link
Copy Markdown
@cloudflare/autoconfig

npm i https://pkg.pr.new/@cloudflare/autoconfig@14652

create-cloudflare

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

@cloudflare/deploy-helpers

npm i https://pkg.pr.new/@cloudflare/deploy-helpers@14652

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-auth

npm i https://pkg.pr.new/@cloudflare/workers-auth@14652

@cloudflare/workers-editor-shared

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

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@14652

wrangler

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

commit: a1d707d

@jamesopstad
jamesopstad force-pushed the james/new-config-runtime-types branch 3 times, most recently from 06a5835 to 08b5881 Compare July 10, 2026 20:29
@jamesopstad
jamesopstad force-pushed the james/new-config-runtime-types branch from 08b5881 to a1d707d Compare July 13, 2026 10:19
@jamesopstad
jamesopstad marked this pull request as ready for review July 13, 2026 10:43
@workers-devprod
workers-devprod requested review from a team and dario-piotrowicz and removed request for a team July 13, 2026 10:43
@workers-devprod

workers-devprod commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@dario-piotrowicz dario-piotrowicz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me 🙂

@workers-devprod workers-devprod left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Codeowners reviews satisfied

@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk Jul 13, 2026
@jamesopstad
jamesopstad merged commit 317ce1f into main Jul 13, 2026
73 of 74 checks passed
@jamesopstad
jamesopstad deleted the james/new-config-runtime-types branch July 13, 2026 15:39
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk Jul 13, 2026
tpmmorris pushed a commit to tpmmorris/workers-sdk that referenced this pull request Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants