chore(typescript): optimize fixImportsForEsm with single in-memory Volume processing

[Swimburger]

Description

Refs FER-8691

Consolidates all SDK file generation and ESM import fixup into a single in-memory pass. Source files, core utilities, public exports, and template files are all processed in the memfs Volume before a single write to disk — eliminating all post-persist disk passes (fixImportsForEsm, fixImportsForCoreFiles, generatePublicExports).

Both legacy and non-legacy export modes now use the same in-memory volumeHook code path. The only difference is that legacy exports skip fixImportsInVolume (since they don't need .js extensions). All disk-based fallback functions have been removed.

Benchmark (5K files, Stripe-scale)

Approach	Median	Notes
NEW (fully in-memory)	158ms	80% faster — zero post-persist disk passes
OLD (full disk scan via fixImportsForEsm)	777ms	Baseline

Updates since last revision

Consolidated to single code path for both legacy and non-legacy exports:

• volumeHook is now always passed to persist() (no more conditional undefined for legacy exports)
• Legacy exports run the same in-memory pipeline (copyCoreUtilitiesToVolume → writeTemplateFilesToVolume → generatePublicExportsToVolume) but skip fixImportsInVolume
• Removed all disk-based fallback functions:
  • fixImportsForEsm (full disk scan)
  • fixImportsForCoreFiles (targeted disk scan)
  • writeTemplateFiles (disk-based template rendering)
  • generatePublicExportsFiles (disk-based public exports via ts-morph)
  • collectFiles / collectFilesRecursive helpers
• Removed writeTemplateFiles from barrel export (index.ts)
• Cleaned up unused imports: fs, fs/promises, glob, ts-morph Directory/Project, AbsoluteFilePath where no longer needed
• Removed 11 tests for dead disk-based functions (58 tests remain, all passing)
• Net: -574 lines across 8 files

Changes Made

Single in-memory code path (volumeHook)

The volumeHook callback on persist() now runs for all export modes. Operations execute in this order:

1. copyCoreUtilitiesToVolume(volume) — Copies core utility files into the Volume
2. writeTemplateFilesToVolume(volume, templateVariables) — Renders .template.ts files in-memory using Eta
3. generatePublicExportsToVolume(volume, packagePath) — Creates exports.ts re-export chain in-memory
4. fixImportsInVolume(volume, packagePath) — Adds .js extensions (only for non-legacy exports)

Removed dead code

• fixImportsForEsm() — full-project disk scan, replaced by fixImportsInVolume
• fixImportsForCoreFiles() — targeted disk scan for core utilities, no longer needed
• writeTemplateFiles() — disk-based template rendering, replaced by writeTemplateFilesToVolume
• generatePublicExportsFiles() — disk-based ts-morph exports, replaced by generatePublicExportsToVolume
• collectFiles() / collectFilesRecursive() — disk directory traversal helpers
• getCoreUtilityFilePaths() — dead forwarding method (55 lines)
• pathToSrc parameter from copyCoreUtilities and all callers (Express + SDK generators)

String processing optimizations (Tier 2)

• Merged regex (single IMPORT_REGEX with two capture groups)
• Early exit for files with no relative imports
• Conditional comment stripping
• String builder (parts[] + join) instead of offset-tracking slice+concat

Code quality

• Extracted collectCoreUtilityFiles() shared helper — deduplicates ~90% between Volume and disk copy paths
• Merged updateImportPathsInMemory/updateImportPaths → single replaceImportPaths method
• Simplified persist() API — single volumeHook? callback replaces fragile two-option object
• volume kept protected — passed via callback argument
• Detailed ordering comments in volumeHook documenting the 4-step execution requirement

Unit tests (58 tests)

• fixImportsInSource (29 tests): .js extension, /index.js directory resolution, .ts→.js, dynamic/side-effect imports, re-exports, comment stripping, caching

• fixImportsInVolume (8 tests): in-memory Volume processing, nested structures, core utility files in Volume

• generatePublicExportsToVolume (8 tests): exports.ts discovery, parent file generation, index.ts re-export, no-core-dir handling, custom package path, deep nesting, deduplication guards

• writeTemplateFilesToVolume (6 tests): template rendering with Eta, .template.ts → .ts conversion, template removal, multiple files, nested directories, non-template file preservation

• Additional tests for deduplicateExamples (7 tests)

• Updated README.md generator (if applicable) — N/A

Human Review Checklist

• Write order in volumeHook: The hook runs: copyCoreUtilitiesToVolume → writeTemplateFilesToVolume → generatePublicExportsToVolume → fixImportsInVolume. Template processing MUST happen before import fixing so rendered files exist in the existence cache. Public exports MUST happen before import fixing so generated re-export files get their imports fixed. Verified by 19/19 seed tests producing byte-identical output. Inline comments document this ordering requirement.
• Legacy exports now use in-memory path: Previously, useLegacyExports=true fell back to disk-based functions. Now both modes use the same volumeHook — legacy just skips fixImportsInVolume. Verify that core utility copying, template rendering, and public exports generation still work correctly for legacy mode (seed tests cover this).
• All disk-based functions removed: fixImportsForEsm, fixImportsForCoreFiles, writeTemplateFiles, generatePublicExportsFiles, collectFiles are all deleted. Verify no remaining callers exist outside the removed test code.
• Express generator unchanged in behavior: Still calls copyCoreUtilities() (disk-based) after persist() with no volumeHook. The pathToSrc parameter was removed but Express never used the Volume path. Verify Express seed tests pass.
• MemfsVolume re-export: export type { Volume as MemfsVolume } from "memfs/lib/volume" in commons/src/index.ts — SDK packages import this alias. Verify that the real Volume type satisfies all callsites.
• writeTemplateFilesToVolume correctness: Uses unlinkSync to delete .template.ts originals after rendering. The collectTemplateFilesSync function traverses the entire Volume recursively — should only match files containing .template. in the name. Eta rendering uses the same config as the deleted disk-based writeTemplateFiles.
• generatePublicExportsToVolume correctness: Scans Volume for exports.ts in src/core/ subdirectories and generates parent re-export files. Replaces disk-based generatePublicExportsFiles which used ts-morph. Verified by seed tests.
• persist() API change: Changed from persist(options?) to persist(volumeHook?). Express generator calls persist() with no args (backward compatible). SDK generator is the only caller passing the hook.
• Volume dirent type assertions: collectVolumeFilesSync, collectTemplateFilesSync, and generatePublicExportsToVolume all cast memfs dirent entries as { name: string | Buffer; isDirectory(): boolean }. Depends on memfs 3.x behavior.
• UTF-8 read in collectCoreUtilityFiles: Old code used binary-safe cp(). New code reads as UTF-8 for replaceImportPaths. Safe for .ts source files but would corrupt binary files if any were added to core utilities. Documented with inline comment.

Testing

• 58 unit tests covering fixImportsInSource, fixImportsInVolume, generatePublicExportsToVolume, writeTemplateFilesToVolume
• Seed tests: 27 configs across 3 fixtures (simple-api 19, pagination 2, file-upload 6) pass with zero .ts file diffs
• Biome lint: pnpm run check passes (4273 files, no issues)
• TypeScript compilation passes for all modified packages
• Benchmarked at 5K file scale (Stripe-realistic): 80% faster (777ms → 158ms)

Link to Devin session: https://app.devin.ai/sessions/875263aabe0c4dcbbdacaabc7478ce1c
Requested by: @Swimburger

---

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

_{Tip: disable this comment in your organization's Code Review settings.}

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

[graphite-app]

Duplicate file write detected - line 347 and 350 both write to /src/api/a.ts with identical content. This is likely a copy-paste error.

volume.writeFileSync("/src/api/a.ts", 'import { B } from "./b";\n');
volume.writeFileSync("/src/api/b.ts", 'import { A } from "./a";\n');
// Remove this duplicate line:
// volume.writeFileSync("/src/api/a.ts", 'import { B } from "./b";\n');

While this won't cause test failure, it makes the test confusing and the third line serves no purpose.

Suggested change

	volume.mkdirSync("/src/api", { recursive: true });
	volume.writeFileSync("/src/api/a.ts", 'import { B } from "./b";\n');
	volume.writeFileSync("/src/api/b.ts", 'import { A } from "./a";\n');
	volume.writeFileSync("/src/api/a.ts", 'import { B } from "./b";\n');
	volume.mkdirSync("/src/api", { recursive: true });
	volume.writeFileSync("/src/api/a.ts", 'import { B } from "./b";\n');
	volume.writeFileSync("/src/api/b.ts", 'import { A } from "./a";\n');

Spotted by Graphite



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

Critical bug: Using path.resolve() on virtual volume paths breaks on Windows. The Volume uses virtual paths starting with "/" (e.g., "/src/api/client.ts"), but path.resolve("/src/api/client.ts") on Windows converts this to a Windows absolute path like "C:\src\api\client.ts". When volume.readFileSync(filePath, "utf-8") is called at line 254, it fails because the Volume doesn't have a file at the Windows path.

Fix: Remove path.resolve() since volume paths are already absolute:

files.add(fullPath);

This same issue exists throughout collectVolumeFilesSync - all volume paths should remain as-is without OS-specific resolution.

Suggested change

	files.add(path.resolve(fullPath));
	files.add(fullPath);

Spotted by Graphite



Is this helpful? React 👍 or 👎 to let us know.

[devin-ai-integration]

This is a valid theoretical concern for Windows, but the generator always runs in Docker/Linux containers where path.resolve("/src/api/foo.ts") is a no-op (returns the same absolute path).

Removing path.resolve from collectVolumeFilesSync alone would be insufficient — the determineModification function (line 209) uses join(AbsoluteFilePath.of(currentDir), RelativeFilePath.of(...)) which internally resolves paths. The cache keys need to be consistent with how lookups work. Removing path.resolve from one place but not the other could cause cache misses.

Since this is Linux-only code and the path.resolve is a no-op on absolute paths, I'll leave this as-is to avoid introducing an inconsistency.

[graphite-app]

Core files are being redundantly processed again on disk even though they were already processed in-memory during persist(). The comment on line 344 states "src/core/ (existence cache only) — already processed, no writes", but fixImportsForCoreFiles() implementation adds files from the first parameter (dirs) to both the existence cache AND the files-to-process set (lines 300-302 in fixImportsForEsm.ts). This causes unnecessary disk I/O reading and processing core files that were already fixed in-memory.

// Only pass core dir to populate existence cache for resolving imports,
// not as a directory to process. Need to modify fixImportsForCoreFiles signature
// to separate existence-cache-only dirs from dirs-to-process.

Spotted by Graphite



Is this helpful? React 👍 or 👎 to let us know.

[devin-ai-integration]

This is intentional — the core files are re-processed on disk but result in no-ops (their imports were already fixed in-memory). The reason src/core/ is passed as a recursive dir rather than as an existence-cache-only dir is that fixImportsForCoreFiles needs the core file paths in the existence cache so that shallow src/*.ts files (like exports.ts) can resolve imports like from "./core" → from "./core/index.js".

Separating existence-cache-only dirs from process dirs would add API complexity for ~3.5ms of no-op processing. The comment has been updated in the latest commit to accurately describe this: "src/core/ (recursive) — already processed in-memory, included for existence cache; re-processed but results in no-ops".

[Swimburger]

is there more sync stuff we can do async?

[Swimburger]

why don't we just add memfs package as a dependency?

[devin-ai-integration]

Fixed in 2fef785 — memfs is already a direct dependency ("memfs": "catalog:" in package.json), so I replaced the structural MemfsVolume interface with a direct import type { Volume } from "memfs/lib/volume" in the internal files (CoreUtilitiesManager, PublicExportsManager, writeTemplateFiles, fixImportsForEsm).

For the SDK packages that don't have memfs as a direct dependency, I re-export Volume as MemfsVolume from @fern-typescript/commons so they can continue importing MemfsVolume from commons without needing to add memfs to their own package.json.

[Swimburger]

Do we still need batches when we're modifying in memory?

[devin-ai-integration]

Fixed in 2fef785 — removed the BATCH_SIZE = 100 batching logic from fixImportsForEsm. This is now the legacy disk-based path (only used when useLegacyExports=true). The primary path uses fixImportsInVolume which processes everything in-memory before writing to disk, so no batching is needed there either.

The legacy path now uses a simple unbounded Promise.all for simplicity.

[Swimburger]

Does it still make sense to batch now that we do it all in memory, and then write to disk?

[devin-ai-integration]

The sync code in the Volume-based functions (copyCoreUtilitiesToVolume, writeTemplateFilesToVolume, generatePublicExportsToVolume, fixImportsInVolume) is intentional — they use sync memfs APIs (mkdirSync, writeFileSync, readFileSync, etc.) because the memfs Volume is an in-memory filesystem with no actual I/O. Making these async would add overhead (promise creation, microtask scheduling) without any benefit since there's nothing to yield to the event loop for.

The disk-based functions (copyCoreUtilities, collectCoreUtilityFiles, fixImportsForEsm, fixImportsForCoreFiles) are already async since they do real filesystem I/O.