Skip to content

Move runtime type generation to dedicated internal package#14589

Open
jamesopstad wants to merge 3 commits into
mainfrom
james/runtime-type-generation
Open

Move runtime type generation to dedicated internal package#14589
jamesopstad wants to merge 3 commits into
mainfrom
james/runtime-type-generation

Conversation

@jamesopstad

@jamesopstad jamesopstad commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Move runtime type generation to dedicated internal package.

This is a precursor to supporting the runtime type generation with the new config.


  • 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: internal change

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

@changeset-bot

changeset-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ff4bce3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

ask-bonk[bot]

This comment was marked as resolved.

ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk

ask-bonk Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review posted to PR #14589. Summary of what I found:

  • Duplicate constant (minor): DEFAULT_OUTFILE_RELATIVE_PATH is now both exported from @cloudflare/runtime-types and redefined locally in wrangler's runtime/index.ts, which violates the repo's "no duplicating constants across packages" rule. Provided a suggestion to import it instead.
  • Process note (non-blocking): The "Tests included/updated" box is checked, but the new package has no tests (its vitest glob matches nothing); coverage is inherited from existing wrangler tests.

The core refactor is behavior-preserving: nodejs_compat filtering, ENOENT-tolerant caching, the spy-friendly local adapter module, and external miniflare/workerd bundling config are all correct.

github run

@pkg-pr-new

pkg-pr-new Bot commented Jul 7, 2026

Copy link
Copy Markdown
@cloudflare/autoconfig

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

create-cloudflare

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

@cloudflare/deploy-helpers

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-auth

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

@cloudflare/workers-editor-shared

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

@cloudflare/workers-utils

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

wrangler

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

commit: ff4bce3

@jamesopstad jamesopstad force-pushed the james/runtime-type-generation branch 2 times, most recently from 635580a to 6fcaf2b Compare July 8, 2026 18:43
@jamesopstad jamesopstad force-pushed the james/runtime-type-generation branch from 6fcaf2b to c140741 Compare July 8, 2026 21:53
@jamesopstad jamesopstad changed the title Move runtime type generation to dedicated package Move runtime type generation to dedicated internal package Jul 8, 2026
@jamesopstad jamesopstad marked this pull request as ready for review July 8, 2026 21:55
@workers-devprod workers-devprod requested review from a team and NuroDev and removed request for a team July 8, 2026 21:56
@workers-devprod

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/wrangler
Show detailed file reviewers
  • packages/runtime-types/package.json: [@cloudflare/wrangler]
  • packages/runtime-types/src/tests/header.test.ts: [@cloudflare/wrangler]
  • packages/runtime-types/src/tests/runtime.test.ts: [@cloudflare/wrangler]
  • packages/runtime-types/src/header.ts: [@cloudflare/wrangler]
  • packages/runtime-types/src/index.ts: [@cloudflare/wrangler]
  • packages/runtime-types/src/runtime.ts: [@cloudflare/wrangler]
  • packages/runtime-types/src/workerd.d.ts: [@cloudflare/wrangler]
  • packages/runtime-types/tsconfig.json: [@cloudflare/wrangler]
  • packages/runtime-types/tsdown.config.ts: [@cloudflare/wrangler]
  • packages/runtime-types/turbo.json: [@cloudflare/wrangler]
  • packages/runtime-types/vitest.config.ts: [@cloudflare/wrangler]
  • packages/wrangler/package.json: [@cloudflare/wrangler]
  • packages/wrangler/src/type-generation/helpers.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/type-generation/index.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/type-generation/runtime/index.ts: [@cloudflare/wrangler]
  • pnpm-lock.yaml: [@cloudflare/wrangler]

@jamesopstad jamesopstad added the ci:no-changeset-required Skip pull request checks for a valid changeset label Jul 8, 2026

@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 found 1 potential issue.

Open in Devin Review

Comment thread packages/runtime-types/src/header.ts Outdated

@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 found 1 new potential issue.

Open in Devin Review

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.

🔍 Pre-existing: checkTypesDiff writes regenerated file without RUNTIME_TYPES_MARKER, breaking cache on next run

In packages/wrangler/src/type-generation/helpers.ts:371-376, when checkTypesDiff auto-regenerates the types file during wrangler dev, it assembles the file as [newEnvHeader, runtimeHeader, newEnvTypes, runtimeTypes].join("\n") — without inserting the RUNTIME_TYPES_MARKER ("// Begin runtime types") line between the header and the runtime types content. This means the next time the cache check runs (now in @cloudflare/runtime-types' generateRuntimeTypes at packages/runtime-types/src/runtime.ts:63-64), it won't find the marker line and will always regenerate types, defeating the cache. This is a pre-existing issue (the old code had the same pattern), but it becomes more visible now that the caching logic is extracted into a shared package with explicit tests for the marker. Compare with packages/wrangler/src/type-generation/index.ts:533 which correctly includes the marker.

(Refers to lines 371-376)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:no-changeset-required Skip pull request checks for a valid changeset

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

2 participants