POC: overwrite.removeStaleFiles ignored for server preset (upstream @graphql-codegen/cli bug) - #488
Draft
eddeee888 wants to merge 2 commits into
Draft
POC: overwrite.removeStaleFiles ignored for server preset (upstream @graphql-codegen/cli bug)#488eddeee888 wants to merge 2 commits into
eddeee888 wants to merge 2 commits into
Conversation
…odegen/cli The server preset hard-codes overwrite.removeStaleFiles=false so watch mode doesn't delete resolver files. But @graphql-codegen/cli's normalizeOverwriteConfig() looks up the matching `generates` entry by an exact match on each generated file's own path, and requires that entry to have a `plugins` key. Preset-based outputs are keyed by baseOutputDir (not per-file) and have no `plugins` key, so the lookup always misses and Codegen silently falls back to the global default (removeStaleFiles: true). Add a pnpm patch for @graphql-codegen/cli@7.3.1 that matches `generates` entries by path-prefix and also recognizes preset-based outputs, plus a test that fails against the unpatched dependency and passes against the patched one. This is a local proof of concept to demonstrate and validate the fix - the real fix still needs to land upstream in graphql-code-generator/graphql-code-generator. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QRhgmWtrzQAFoN47ebXrkY
|
…matching
Replaces the path-prefix lookup with the approach codegen.js already uses for
`hooks`: `process` in codegen.js has both the `generates` key and the per-file
path in scope, so it now tags each result with that entry's `overwrite`.
`normalizeOverwriteConfig` then reads `fileOutput.overwrite` directly and needs
no lookup, so `findOutputConfig` and the `plugins`-only `isConfiguredOutput`
gate are both gone - the function shrinks rather than grows.
Prefix matching was also not just inelegant but wrong: a preset can emit files
whose paths are not under its baseOutputDir at all (the server preset does),
and those would still have missed.
`removeStaleFiles` now retains the previous run's {filename, overwrite} pairs so
a file that disappears is judged by the entry that produced it.
Tests exercise the CLI contract with a stub preset - a directory-keyed entry
with no `plugins` key emitting a path outside its base dir - keeping them
independent of the server preset's internals and of graphql module identity.
All four fail against the unpatched dependency and pass against the patched one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QRhgmWtrzQAFoN47ebXrkY
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The issue
The server preset hard-codes
overwrite.removeStaleFiles: false(packages/typescript-resolver-files/src/defineConfig.ts) so that watch mode doesn't delete resolver files. That setting never takes effect.The cause is in
@graphql-codegen/cli@7.3.1,generate-and-save.js:Two separate assumptions break for a preset-based output:
normalizeOverwriteConfig(config, filename)for each generated file, but a preset'sgeneratesentry is keyed by itsbaseOutputDir. Nogenerateskey ever equals the path of a file the preset emitted, so the lookup always misses.isConfiguredOutputrequiresplugins. Even on a hit, a preset entry has nopluginskey — the preset supplies plugins internally — so it isn't recognised as a configured output.Both paths
return result, the globalconfig.overwrite, which defaults totrue→removeStaleFiles: true. In watch mode that deletes the very resolver files the preset'sfalsewas meant to protect.updateExistingFiles: trueis dropped identically; it just happens to match the fallback default, so onlyremoveStaleFilesis observable.For the record,
overwrite.checkStaleFilesis not a real option at all —NormalizedOverwriteOptionin@graphql-codegen/plugin-helpershas exactlyremoveStaleFilesandupdateExistingFiles. And this is not something@eddeee888/gcg-server-configcan influence: it produces plugin config, whileoverwriteis an output-level field.The patch
patches/@graphql-codegen__cli@7.3.1.patch, applied viapnpm patch/patchedDependencies.codegen.jsalready copieshooksfrom the output config onto every generated file. Itsprocessfunction has both thegenerateskey and the per-file path in scope, sooverwritecan ride along the same way:normalizeOverwriteConfigthen reads the file's own value and needs no lookup:That deletes the
config.generateslookup andisConfiguredOutputentirely — the function gets smaller, and both failure modes above disappear at once.removeStaleFilesretains the previous run's{ filename, overwrite }pairs so a file that disappears is still judged by the entry that produced it.Matching
generateskeys by path-prefix was considered and rejected: besides being guesswork, it's wrong. A preset can emit files that aren't under itsbaseOutputDirat all — the server preset does, e.g.resolvers/User.ts— and those would still have missed. Tagging at the source has no such failure mode.Tests are in
packages/typescript-resolver-files/src/defineConfig.staleFilesCliBug.spec.ts: one drives the publicexecuteCodegenAPI and asserts every file a preset emits carries the entry'soverwrite; three covernormalizeOverwriteConfig's resolution rules (file value wins, global fallback, boolean shorthand). The propagation test uses a stub preset shaped like the server preset — directory-keyed entry, nopluginskey, one emitted path deliberately outside the base dir — so it stays independent of the server preset's internals. All four fail against the unpatched dependency and pass against the patched one; the full suite is green at 129 tests.This is a PoC — the real fix goes upstream
A
pnpm patchonly affects this repo's ownnode_modules. It does not reach consumers of@eddeee888/gcg-typescript-resolver-files, whose own@graphql-codegen/cliinstall is untouched. This branch exists to pin down the root cause and prove the fix works end to end.The fix belongs in graphql-code-generator/graphql-code-generator and needs to ship in a new
@graphql-codegen/clirelease before users benefit. When upstreaming,Types.FileOutputlives in@graphql-codegen/plugin-helpersand will need an optionaloverwrite?: boolean | Partial<NormalizedOverwriteOption>field — not patched here, since this PoC only changes the CLI's runtime JS.Draft until we decide whether to keep the patch as an interim measure or go straight upstream.