refactor(tsmorph): remove duplicated import builders and unreachable guards in generateFiles - #207
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
7nohe
force-pushed
the
claude/lucid-heisenberg-ed96ff
branch
from
August 11, 2026 11:39
e991da8 to
f795136
Compare
… guards `generateFiles.mts` kept a verbatim private copy of `buildCommonFileImports` and `buildHookFileImports`, duplicating the exported versions in `projectFactory.mts`. The exported ones are covered by `tests/tsmorph/projectFactory.test.ts`; the copies were not, so the duplicated code was also the untested code. Import them instead. Three `if (hook)` null guards were unreachable: each call site pre-filtered on `isPaginatable`, and `buildInfiniteHook` / `buildPrefetchInfiniteQueryFn` return null only when `!op.isPaginatable`. Rather than assert non-null, map over the GET operations and drop the nulls, which keeps the types honest and leaves the builder as the single source of truth for pagination. The `if (infiniteOptions)` guard in `generateQueryOptionsFile` is left alone: that call site does not pre-filter and both paths are exercised. Generated example output is byte-identical. Branch coverage: generateFiles.mts 68.75% -> 75%, repo aggregate 90.07% -> 91.25%.
…actory `generateQueryOptionsFile` assembled its own import list inline, repeating the `buildModelImport` + null-check pattern that `buildCommonFileImports` already owns. Like the copies removed in the previous commit, that inline branch was the one part of the import wiring no test reached. Move it next to its siblings in `projectFactory.mts` and cover both the with-models and no-models paths, mirroring the existing `buildCommonFileImports` tests. Generated example output is byte-identical. Branch coverage: generateFiles.mts 75% -> 100%, repo aggregate 91.25% -> 91.66%.
…helper
generateFiles.mts had four call sites for builders that return null when the
operation is out of scope, written three different ways: a plain `if` guard for
`buildInfiniteQueryOptionsFn`, and three near-identical
`.map(build).filter(x => x !== null)` blocks each carrying its own near-identical
three-line comment.
Collapse all four onto a single `addStatements(sourceFile, operations, build)`
helper. The contract ("a builder returns null for operations outside its scope")
is now stated once, in the helper's JSDoc, instead of being re-derived at every
site.
This also fixes a build regression: narrowing `(T | null)[]` via a bare
`.filter(x => x !== null)` relies on inferred type predicates, a TypeScript 5.5
feature, while package.json declares a `typescript: "5.x || 6.x"` peer range.
Verified with typescript@5.4.5 -- `tsc --noEmit` failed with three TS2345 errors
before this commit and passes after. Consumers of the published `dist` were
never affected; this only broke building from source on TS 5.0-5.4.
The queryOptions import test now asserts the exact module-specifier order rather
than mere membership, since that order is emitted verbatim.
Generated example output is byte-identical.
7nohe
force-pushed
the
claude/lucid-heisenberg-ed96ff
branch
from
August 11, 2026 11:42
f795136 to
5f98c3c
Compare
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.
Follow-up to the
buildKeysdead-code removal (#205), kept out of that PR deliberately. Rebased onto currentmain(includes #205 and #204).src/tsmorph/generateFiles.mtswas the worst-covered file in the repo at 68.75% branch coverage, and it was uncoverable by adding tests: every uncovered branch was either structurally unreachable or a duplicate of already-tested code. This removes those branches instead.1. Duplicated import builders
generateFiles.mtsheld a verbatim private copy ofbuildCommonFileImports/buildHookFileImports, which are already exported and tested inprojectFactory.mts(tests/tsmorph/projectFactory.test.ts). The duplicated code was also the untested code. It now imports them.2. Unreachable null guards
Three
if (hook)guards could never be false — each call site pre-filtered onisPaginatable, and the builders returnnullonly when!op.isPaginatable:generateSuspenseFilebuildUseSuspenseInfiniteQueryHook→buildInfiniteHookbuildQueryHooks.mts:271generateInfiniteQueriesFilebuildUseInfiniteQueryHook→buildInfiniteHookbuildQueryHooks.mts:271generatePrefetchFilebuildPrefetchInfiniteQueryFnbuildQueryHooks.mts:3813. Extracted
buildQueryOptionsFileImportsWith 1 and 2 done, one uncovered branch remained: the inline
buildModelImport+ null-check ingenerateQueryOptionsFile. Same defect class as 1 — an ad-hoc import list duplicating logicprojectFactory.mtsalready owns, and the only part of the import wiring no test reached. Moved next to its siblings and covered both the with-models and no-models paths.4. One
addStatementshelper for every nullable builderThe first pass left
generateFiles.mtswith four nullable-builder call sites written three different ways. All four now go through one helper:The contract — a builder returns null for operations outside its scope — is stated once in the helper's JSDoc instead of being re-derived at each site, and three near-identical explanatory comments collapse to one.
This also fixes a build regression the earlier pass introduced: narrowing
(T | null)[]with a bare.filter(x => x !== null)relies on inferred type predicates, a TypeScript 5.5 feature, whilepackage.jsondeclares atypescript: "5.x || 6.x"peer range. Verified againsttypescript@5.4.5:tsc --noEmitproduced threeTS2345errors before this commit and exits 0 after. Consumers of the publisheddistwere never affected — this only broke building from source on TS 5.0–5.4.Note this brings
generateQueryOptionsFile'sif (infiniteOptions)guard along too. That guard is genuinely load-bearing (its call site does not pre-filter) and is not removed — the same conditional now lives inside the helper, exercised both ways.Coverage
generateFiles.mtsbranchesVerification
npm test— 189 passed, exit 0, unmodified confignpm run build,npm run lint— clean (107 files, zero warnings)tsc --noEmitundertypescript@5.4.5and@6.0.3— both cleannpm run preview:reactdiffed against a baseline generated frommain: byte-identicalNote on the
perFilefollow-upThe motivation was to enable
coverage.thresholds.perFile: trueinvitest.config.ts, since the global aggregate can be moved by adding or deleting any file regardless of whether behaviour got safer.generateFiles.mtsis no longer the blocker, butperFile: truestill cannot be enabled —src/parseOperations.mtsfails it on functions, statements and branches:So
vitest.config.tsis deliberately untouched. CoveringparseOperations.mts(uncovered lines 27, 43, 55) is the remaining prerequisite.