[wrangler] fix: ensure types --check --env-file does not falsely report stale when .dev.vars exists#13751
Conversation
🦋 Changeset detectedLatest commit: 03c25ec The changes in this PR will be included in the next version bump. 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 |
workers-devprod
requested review from
a team and
jamesopstad
and removed request for
a team and
Copilot
|
Codeowners approval required for this PR:
Show detailed file reviewers |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
NuroDev
left a comment
NuroDev
left a comment
There was a problem hiding this comment.
Overall this change looks good to me, thanks @matingathani!
Just a few file conflicts that need to be fixed up and should be good to merge
|
Regarding the Also moved the |
matingathani
force-pushed
the
fix/wrangler-types-check-envfile
branch
from
720ea61 to
ebef732
Compare
workers-devprod
left a comment
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
There was a problem hiding this comment.
Pull request overview
Fixes a Wrangler types --check regression where specifying --env-file was not respected during the internal regeneration step, causing .dev.vars to be loaded and the generated hash to mismatch.
Changes:
- Pass the active CLI
--env-filethrough thetypes --checkflow intocheckTypesUpToDate. - Teach
checkTypesUpToDateto prefer the CLI-providedenvFile, falling back to parsing it from the generated header command. - Add a regression test covering
types --check --env-file ...when.dev.varsexists, plus a changeset for a patch release.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/wrangler/src/type-generation/index.ts | Threads args.envFile into checkTypesUpToDate for --check. |
| packages/wrangler/src/type-generation/helpers.ts | Extends checkTypesUpToDate to carry envFile into generateEnvTypes during re-check. |
| packages/wrangler/src/tests/type-generation.test.ts | Adds regression coverage for --check --env-file when .dev.vars is present. |
| .changeset/fix-wrangler-types-check-env-file.md | Declares a patch changeset for the fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Fixes an issue in Wrangler’s types --check flow where providing --env-file was not respected during the internal regeneration step, causing false “stale types” reports when a local .dev.vars file exists.
Changes:
- Pass the active CLI invocation’s
envFile/envvalues intocheckTypesUpToDatefrom thetypes --checkhandler. - Teach
checkTypesUpToDateto prefer CLI-providedenvFile/env, falling back to parsing them from the types file’s “Generated by Wrangler…” header. - Add a regression test ensuring
types --check --env-file=<empty file>is considered up-to-date even when.dev.varsexists.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/wrangler/src/type-generation/index.ts | Forwards envFile/env into the --check comparison path so regeneration matches the user’s flags. |
| packages/wrangler/src/type-generation/helpers.ts | Uses CLI-provided envFile/env when recomputing the env hash during --check, preventing .dev.vars from being implicitly loaded when --env-file was intended. |
| packages/wrangler/src/tests/type-generation.test.ts | Adds regression coverage for the .dev.vars + --env-file + --check scenario. |
| .changeset/fix-wrangler-types-check-env-file.md | Ships the fix as a patch changeset for wrangler. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Fixes wrangler types --check --env-file … incorrectly reporting generated types as stale when a local .dev.vars exists, by ensuring the --check regeneration path uses the same --env-file/--env context as the active CLI invocation.
Changes:
- Pass the active CLI invocation’s
envFileandenvvalues intocheckTypesUpToDate(). - Update
checkTypesUpToDate()to prefer CLI-providedenvFile/env, falling back to values parsed from the generated types header. - Add a regression test covering
types --check --env-filebehavior when.dev.varsexists.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/wrangler/src/type-generation/index.ts | Wires args.envFile/args.env through to the --check staleness check so regen uses the correct environment inputs. |
| packages/wrangler/src/type-generation/helpers.ts | Extends checkTypesUpToDate() to carry envFile/env into generateEnvTypes() during --check comparisons. |
| packages/wrangler/src/tests/type-generation.test.ts | Adds regression test ensuring --check --env-file does not become stale due to .dev.vars. |
| .changeset/fix-wrangler-types-check-env-file.md | Adds patch changeset entry documenting the fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
There seems to be some formatting issues that need fixing @matingathani: Running |
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
…rt stale when .dev.vars exists In checkTypesUpToDate, envFile was not extracted from the header comment nor forwarded from the active CLI invocation. This caused --check to always load .dev.vars, producing a hash mismatch when types were generated with --env-file /dev/null. The fix: accept cliEnvFile parameter from the --check handler and use it as the primary source of truth for env file loading during hash comparison. Fixes cloudflare#13679
… for cross-platform compatibility - Add cliEnv parameter to checkTypesUpToDate and thread through to generateEnvTypes to prevent hash mismatches when configs have named environments - Replace /dev/null with an empty temp file in the regression test for Windows compat
…to rawArgs per review
matingathani
force-pushed
the
fix/wrangler-types-check-envfile
branch
from
8c0678f to
45b22cd
Compare
|
Rebased on main and ran |
|
Codeowners approval required for this PR:
Show detailed file reviewers |
|
@NuroDev @penalosa @dario-piotrowicz — all Copilot review comments have been addressed. This PR is ready for review. |
Fixes #13679.
wrangler types --check --env-file /dev/nullfalsely reported types as stale whenever a local.dev.varsexisted, even when the on-disk file was generated with the matchingwrangler types --env-file /dev/nullcommand.The
--checkcodepath incheckTypesUpToDatedroppedenvFilewhen re-running env-type generation internally. This caused.dev.varsto be loaded unconditionally, producing a hash mismatch.Fix:
checkTypesUpToDatenow accepts an optionalcliEnvFileparameter--checkhandler passes the active CLI'senvFiletocheckTypesUpToDatecheckTypesUpToDateuses the CLI-providedenvFileas the primary source of truth, falling back to parsing from the types file header comment--check --env-file /dev/nullreports types up-to-date even when.dev.varsexists