fix(varlock): defer process.env typing when another .d.ts declares ProcessEnv - #1086
theoephraim wants to merge 6 commits into
Conversation
…ocessEnv wrangler types writes a NodeJS.ProcessEnv augmentation into worker-configuration.d.ts. TypeScript merges all declarations of that interface and requires every shared key to be identical, so ours could never coexist with it: an optional item alone (FOO?: string vs FOO: string) fails with TS2320, as do enums and booleans. Default processEnv to none when another .d.ts next to the schema (or the generated file) already declares ProcessEnv, and note which file caused it at the top of the generated output. Detecting the declaration rather than the platform covers infra-as-code setups with no wrangler config, and misses nothing that was actually conflicting. Explicit processEnv= still wins.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
varlock-website | f350959 | Commit Preview URL Branch Preview URL |
Sep 14 2026, 05:33 AM |
|
The changes in this PR will be included in the next version bump.
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The declaration detector can suppress valid process.env typing based on unrelated syntax in a neighboring declaration file.
Reviewed changes in f350959f: the new foreign NodeJS.ProcessEnv detection, its integration with TypeScript generation, public export, generated note, tests, documentation, and release entry.
- Conflict detection: Scans neighboring
.d.tsfile heads while excluding the target output and Varlock-generated files. - Generation behavior: Defaults implicit
processEnvgeneration tononewhen a conflict is found, while preserving explicit overrides and other generated types. - Coverage and guidance: Adds focused detector and generation tests, Cloudflare guidance, decorator reference details, and a patch bump.
azure/gpt-5.6-sol ｜ 𝕏
The two substring checks did not establish that ProcessEnv was declared
inside NodeJS, so `interface ProcessEnvExtra`, a ProcessEnv declared
after the namespace block, or a mention in a comment all read as a
conflict and would have silently dropped valid process.env typing.
Strip comments, then brace-match each `namespace NodeJS {` block and
require the interface inside it.
📦 Bundle size
dist/ only; native binaries are versioned separately and not counted here. |
There was a problem hiding this comment.
Important
The revised detector still lets TypeScript literals alter structural matching, so the original false-positive risk remains.
Reviewed changes since the prior Pullfrog review:
- Scoped declaration matching: Replaced independent substring checks with a namespace-scoped brace walk and comment stripping.
- Regression coverage: Added cases for separate declarations, comments, interface-name prefixes, and nested
declare globaldeclarations.
azure/gpt-5.6-sol ｜ 𝕏
| else if (src[i] === '}') depth--; | ||
| i++; | ||
| } | ||
| if (PROCESS_ENV_INTERFACE.test(src.slice(bodyStart, depth === 0 ? i - 1 : undefined))) return true; |
There was a problem hiding this comment.
The brace walk still treats string and template literal contents as TypeScript syntax. For example, declare namespace NodeJS { const marker: 'interface ProcessEnv'; } returns a conflict, while a '}' literal before a real declaration closes the namespace early, so this can still either suppress valid typing or miss the conflict.
Technical details
# Ignore literal contents while scanning declarations
## Affected sites
- packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts:63-77 - comment stripping and raw brace counting leave string and template literal contents active.
## Required outcome
- Only declaration syntax can satisfy `interface ProcessEnv` or change namespace brace depth.
- Add regression cases for declaration-like text and braces inside literals.
varlock
@varlock/native-helper-darwin
@varlock/native-helper-linux-arm64
@varlock/native-helper-linux-x64
@varlock/native-helper-win32-x64
commit: |
…sEnv A `'}'` literal type inside a `namespace NodeJS` block ended the brace walk early, and a literal spelled `'interface ProcessEnv'` faked a declaration. Strip string and template literals alongside comments before scanning.
There was a problem hiding this comment.
Important
Comment stripping still treats delimiters inside literals as syntax, so the conflict detector can miss real ProcessEnv declarations.
Reviewed changes since the prior Pullfrog review in d1da98a5:
- Literal-aware scanning: Added string and template literal removal before namespace and declaration matching.
- Regression coverage: Added cases for declaration-like text and closing braces inside string literals.
azure/gpt-5.6-sol ｜ 𝕏
Stripping comments before literals let a literal spelled '//' or '/*' hide a real declaration after it; stripping literals first would let a comment's apostrophe do the same. Fold both token classes into one regex so a single left-to-right pass takes whichever opens first.
There was a problem hiding this comment.
Important
The source-order fix addresses the reported comment-delimiter cases, but the broader open literal-scanning concern remains partially unresolved: valid quoted literals with escaped newline continuations are not consumed by the regex, so declaration text inside them can still trigger a false conflict and silently suppress Varlock's process.env typing. The existing thread remains open rather than duplicating that finding inline.
Reviewed changes since the prior Pullfrog review: the scanner now consumes comments and literals in one source-ordered pass and adds regressions for delimiters inside literals and apostrophes inside comments.
- Unified lexical scan: Combined comment and literal alternatives into one global expression so the earliest token opener wins.
- Regression coverage: Added tests for
//and/*inside string literals, plus apostrophes inside line comments.
azure/gpt-5.6-sol ｜ 𝕏
…egex Replace the brace-matching namespace walk with a single regex requiring `interface ProcessEnv` to open the `namespace NodeJS` block, which is what every generator that writes one actually emits. `[^}]` cannot run past the first member, so a ProcessEnv declared after the namespace closes still cannot match. This gives up detecting a declaration that is not the namespace's first member. That is the fail-safe direction: a miss leaves today's behaviour and surfaces as TS2320 with the documented processEnv=none fix, while a false positive would silently drop valid typing.
There was a problem hiding this comment.
Important
The adjacency regex does not resolve the existing escaped-newline literal finding. A production-path reproduction still identifies a .d.ts where interface ProcessEnv occurs only inside a valid continued string literal, so the existing thread remains open rather than being duplicated inline.
Reviewed changes since the prior Pullfrog review: the detector was simplified to use one bounded namespace-to-interface regex, with an explicit miss bias for declarations it cannot classify confidently.
-
Replaced the brace walk: Matched
namespace NodeJSand a nearbyinterface ProcessEnvin one expression that cannot cross a closing brace. -
Documented intentional misses: Added coverage showing that a
ProcessEnvdeclaration after a braced namespace member is left for TypeScript and the documentedprocessEnv=noneoverride.
azure/gpt-5.6-sol ｜ 𝕏
…nning A backslash-newline pair ended the literal match, exposing the rest of the literal as if it were syntax, so declaration text inside a continued string literal read as a real ProcessEnv declaration. Let the escape alternative take any character, newline included.
There was a problem hiding this comment.
Important
The LF continuation fix leaves the existing literal-scanning concern open for CRLF source files. TypeScript accepts a backslash followed by CRLF as a line continuation, but \\[\s\S] consumes only \r; the remaining \n terminates the quoted-literal match, and a production-path probe still falsely detected declaration text inside that literal.
Reviewed changes since the prior Pullfrog review in 3d189b0d: the scanner now keeps LF escaped-newline continuations inside quoted literals instead of exposing their contents.
- Expanded escape matching: Changed quoted and template literal escape branches to consume any escaped character, including LF, and added a focused continued-string regression.
azure/gpt-5.6-sol ｜ 𝕏
1 participant
wrangler typeswrites aNodeJS.ProcessEnvaugmentation intoworker-configuration.d.ts. TypeScript merges all declarations of that interface and requires every shared key to be identical, so varlock's augmentation could never coexist with it. A single optional item (FOO?: stringvs wrangler'sFOO: string) is enough to fail with TS2320, as are enums and booleans, which we type as literal unions. UnderskipLibCheck: true(on in most Worker templates) the error is suppressed and whichever declaration TypeScript reaches first silently wins instead.@generateTsTypesnow defaultsprocessEnvtononewhen another.d.tsnext to the schema (or next to the generated file) already declaresNodeJS.ProcessEnv, and the generated file says which file caused it. An explicitprocessEnv=still wins.Detection targets the declaration rather than the platform: infra-as-code setups may have no wrangler config at all, and a project with no other ProcessEnv declaration has no conflict to avoid, so missing one is the safe direction. Our own generated files are skipped via the autogenerated banner, so a second
@generateTsTypeswriting into the same directory isn't mistaken for a foreign declaration. Only the head of each file is read, since wrangler puts ~1MB of runtime types after the env block and codegen runs on every load.Nothing is lost for anyone using
varlock-wrangler types: it feeds the schema's keys to wrangler, so they're all typed onprocess.envas strings, andENVkeeps the coerced types either way.Verified with tsc, not just unit tests: a schema with an enum plus an optional item alongside a real-shaped
worker-configuration.d.tsnow typechecks clean underskipLibCheck: false, and flipping the same project toprocessEnv=strictreproduces TS2320.