fix(orval): keep generated barrels idempotent across formatter runs (#3756) - #3762
Conversation
…rval-labs#3756) The workspace barrel and zod verb-schema index deduped re-exports by matching a single-quote formatted line. An `afterAllFilesWrite` formatter flipping quote style (e.g. prettier single -> double) defeated the check, so every export was re-appended on each generation. Dedup on the bare module specifier instead of the formatted line, via a shared quote-agnostic readReExportSpecifiers helper. The workspace barrel and operationSchemas re-export keep append semantics (the barrel can share its path with the target, orval-labs#3675), while the zod index rebuilds the pure barrel. All three sites share the one extractor.
📝 WalkthroughWalkthroughChangesThe change adds quote-agnostic barrel export parsing and merging, applies deduplication to schema and workspace index generation, and adds regression coverage for repeated generation with formatter quote changes. Barrel export idempotency
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/orval/src/write-specs.ts (1)
172-189: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared "append missing re-export specifiers" helper.
addOperationSchemasReExportand the workspace barrel block both independently implement "read existing barrel → compute declared specifiers viareadReExportSpecifiers→ filter to missing → append" inline. Given this PR's goal is eliminating divergent duplicate-detection logic (the root cause of#3756), keeping two separate inline copies risks the same kind of drift recurring.
packages/orval/src/write-specs.ts#L172-L189: extract the "declared-set check + conditional append" into a shared helper alongsidemergeBarrelSpecifiersinbarrel.ts, and call it here.packages/orval/src/write-specs.ts#L805-L833: call the same shared helper here instead of re-derivingdeclared/toAddinline.♻️ Example shared helper (in barrel.ts)
export async function appendMissingReExports( filePath: string, specifiers: string[], ): Promise<void> { if (await fs.pathExists(filePath)) { const declared = readReExportSpecifiers(await fs.readFile(filePath, 'utf8')); const toAdd = [...new Set(specifiers.filter((s) => !declared.has(s)))]; if (toAdd.length > 0) { await fs.appendFile( filePath, toAdd.map((s) => `export * from '${s}';\n`).join(''), ); } } else { await fs.outputFile( filePath, `${[...new Set(specifiers)].map((s) => `export * from '${s}';`).join('\n')}\n`, ); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/orval/src/write-specs.ts` around lines 172 - 189, Extract the shared “append missing re-export specifiers” logic into a helper alongside mergeBarrelSpecifiers in barrel.ts, reusing readReExportSpecifiers and deduplicating specifiers. Update packages/orval/src/write-specs.ts lines 172-189 to call the helper, and update lines 805-833 to call the same helper instead of computing declared/toAdd inline; preserve existing file-creation and append behavior at both sites.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/orval/src/write-specs.ts`:
- Around line 172-189: Extract the shared “append missing re-export specifiers”
logic into a helper alongside mergeBarrelSpecifiers in barrel.ts, reusing
readReExportSpecifiers and deduplicating specifiers. Update
packages/orval/src/write-specs.ts lines 172-189 to call the helper, and update
lines 805-833 to call the same helper instead of computing declared/toAdd
inline; preserve existing file-creation and append behavior at both sites.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6f5e7e17-7bb6-4e1d-aea3-7539491c0fe3
📒 Files selected for processing (5)
packages/orval/src/generate-spec.test.tspackages/orval/src/utils/barrel.tspackages/orval/src/utils/index.tspackages/orval/src/write-specs.tspackages/orval/src/write-zod-specs.ts
@orval/angular
@orval/axios
@orval/core
@orval/effect
@orval/fetch
@orval/hono
@orval/mcp
@orval/mock
orval
@orval/query
@orval/solid-start
@orval/swr
@orval/zod
commit: |
Fixes #3756.
Problem
The shared workspace barrel (
<workspace>/index.ts) and the zod verb-schema index accumulated duplicateexport *lines on every regeneration when anafterAllFilesWriteformatter (e.g. prettier) ran between generations. Each run re-appended the full export set instead of being a no-op.Root cause
Both barrels deduped re-exports by matching a single-quote formatted line (
data.includes(export * from '${imp}')). orval writes single quotes; a formatter flips them to double quotes; the next run's substring check no longer matches → every export is re-appended. Unbounded growth.Fix
Dedup on the bare module specifier (not the formatted line) via a shared quote-agnostic
readReExportSpecifiershelper (packages/orval/src/utils/barrel.ts):targetisindex.ts#3675:target === <workspace>/index.ts);mergeBarrelSpecifiershelper backs this).All three barrel re-export sites now share the one extractor; the operationSchemas site's duplicated inline regex is removed.
Verification
generate-spec.test.ts): two projects sharing a workspace barrel, regenerate with a simulated quote flip between runs → asserts no accumulation. Fails on master, passes with the fix.issue-3675-index-target(barrel sharing the target path) confirmed preserved.workspace,afterAllFilesWrite: prettier, 2 projects):src/index.tswent 4 → 8 → 12 lines across 3 runs, now stable at 4.Summary by CodeRabbit