-
-
Notifications
You must be signed in to change notification settings - Fork 0
perf(move-file): cache index file export analysis #316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
ff7458e
d0358cd
a188b59
6908df5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| ÔùÅ Validation Warning: | ||
|
|
||
| Unknown option "reporters" with value ["default"] was found. | ||
| This is probably a typing mistake. Fixing it will remove this message. | ||
|
|
||
| Configuration Documentation: | ||
| https://jestjs.io/docs/configuration | ||
|
|
||
| console.log | ||
| [Export Management > Export detection] should detect exports x 56,768 ops/sec ┬▒2.09% (48553 runs sampled) | ||
|
|
||
| at Object.<anonymous> (tools/tinybench-utils.ts:1323:17) | ||
|
|
||
| console.log | ||
| [Export Management > Export addition] should add exports x 61,458 ops/sec ┬▒1.81% (54844 runs sampled) | ||
|
|
||
| at Object.<anonymous> (tools/tinybench-utils.ts:1323:17) | ||
|
|
||
| console.log | ||
| [Export Management > Export removal] should remove exports x 56,077 ops/sec ┬▒1.82% (50026 runs sampled) | ||
|
|
||
| at Object.<anonymous> (tools/tinybench-utils.ts:1323:17) | ||
|
|
||
| PASS benchmarks packages/workspace/src/generators/move-file/benchmarks/export-management.bench.ts | ||
| Export Management | ||
| Export detection | ||
|  should detect exports (25 ms) | ||
| Export addition | ||
|  should add exports (1 ms) | ||
| Export removal | ||
|  should remove exports | ||
|
|
||
| Test Suites: 1 passed, 1 total | ||
| Tests: 3 passed, 3 total | ||
| Snapshots: 0 total | ||
| Time: 4.658 s, estimated 5 s | ||
| Ran all test suites matching /export-management/i. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,69 @@ | ||||||||||||||||||||||||
| import type { Tree } from '@nx/devkit'; | ||||||||||||||||||||||||
| import { treeReadCache } from '../tree-cache'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Cached export information for index (entrypoint) files to avoid reparsing. | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| export interface IndexExports { | ||||||||||||||||||||||||
| exports: Set<string>; // direct exports (file paths without extension) | ||||||||||||||||||||||||
| reexports: Set<string>; // re-exported modules (file paths without extension) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| interface CachedIndexExports extends IndexExports { | ||||||||||||||||||||||||
| content: string; // content snapshot used to build this cache entry | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Internal cache keyed by normalized index file path | ||||||||||||||||||||||||
| const indexExportsCache = new Map<string, CachedIndexExports>(); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** Clears all cached index export data. */ | ||||||||||||||||||||||||
| export function clearIndexExportsCache(): void { | ||||||||||||||||||||||||
| indexExportsCache.clear(); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** Invalidates a single index file from the cache (e.g., after write). */ | ||||||||||||||||||||||||
| export function invalidateIndexExportsCacheEntry(indexPath: string): void { | ||||||||||||||||||||||||
| indexExportsCache.delete(indexPath); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Get (and cache) export info for an index/entrypoint file. | ||||||||||||||||||||||||
| * Lightweight regex based extraction – sufficient for current export patterns. | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| export function getIndexExports(tree: Tree, indexPath: string): IndexExports { | ||||||||||||||||||||||||
| const content = treeReadCache.read(tree, indexPath, 'utf-8') || ''; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const cached = indexExportsCache.get(indexPath); | ||||||||||||||||||||||||
| if (cached && cached.content === content) return cached; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const exports = new Set<string>(); | ||||||||||||||||||||||||
| const reexports = new Set<string>(); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Match: export * from './path'; OR export { ... } from './path'; OR export {default as X} from './path'; | ||||||||||||||||||||||||
| const reExportPattern = | ||||||||||||||||||||||||
| /export\s+(?:\*|\{[^}]+\})\s+from\s+['"](\.\.?\/[^'";]+)['"];?/g; | ||||||||||||||||||||||||
| // Match: export * from './path'; specially capture star exports for potential future distinction | ||||||||||||||||||||||||
| // Simple capture group for path without extension processing here | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let match: RegExpExecArray | null; | ||||||||||||||||||||||||
| while ((match = reExportPattern.exec(content))) { | ||||||||||||||||||||||||
| const spec = match[1]; | ||||||||||||||||||||||||
| reexports.add(spec); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| reexports.add(spec); | |
| // Remove file extension (e.g., .ts, .js, .jsx, .tsx) from specifier | |
| const specWithoutExt = spec.replace(/\.[tj]sx?$/, ''); | |
| reexports.add(specWithoutExt); |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file extension removal logic duplicates functionality that likely exists elsewhere in the codebase (e.g., removeSourceFileExtension that's imported in the other file). Consider extracting this to a shared utility or reusing existing functionality.
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic adds reexports to the exports set, which conflates two different types of exports. The reexports should remain in the reexports set, and exports should contain actual local exports from the file.
| for (const spec of reexports) { | |
| const withoutExt = spec.replace(/\.(ts|tsx|js|jsx)$/i, ''); | |
| // Ensure stored paths are normalized to start with './' | |
| const normalized = | |
| withoutExt.startsWith('./') || withoutExt.startsWith('../') | |
| ? withoutExt | |
| : `./${withoutExt}`; | |
| exports.add(normalized); | |
| } | |
| // Note: Do not add reexports to exports. Exports should only contain actual local exports. | |
| // If needed, add logic here to capture local exports (e.g., export { foo, bar };) in the future. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| ÔùÅ Validation Warning: | ||
|
|
||
| Unknown option "reporters" with value ["default"] was found. | ||
| This is probably a typing mistake. Fixing it will remove this message. | ||
|
|
||
| Configuration Documentation: | ||
| https://jestjs.io/docs/configuration | ||
|
|
||
| console.log | ||
| [Export Management > Export detection] should detect exports x 62,156 ops/sec ┬▒1.66% (55877 runs sampled) | ||
|
|
||
| at Object.<anonymous> (tools/tinybench-utils.ts:1323:17) | ||
|
|
||
| console.log | ||
| [Export Management > Export addition] should add exports x 61,446 ops/sec ┬▒1.30% (54407 runs sampled) | ||
|
|
||
| at Object.<anonymous> (tools/tinybench-utils.ts:1323:17) | ||
|
|
||
| console.log | ||
| [Export Management > Export removal] should remove exports x 62,213 ops/sec ┬▒4.02% (56818 runs sampled) | ||
|
|
||
| at Object.<anonymous> (tools/tinybench-utils.ts:1323:17) | ||
|
|
||
| PASS benchmarks packages/workspace/src/generators/move-file/benchmarks/export-management.bench.ts | ||
| Export Management | ||
| Export detection | ||
|  should detect exports (13 ms) | ||
| Export addition | ||
|  should add exports (1 ms) | ||
| Export removal | ||
|  should remove exports (1 ms) | ||
|
|
||
| Test Suites: 1 passed, 1 total | ||
| Tests: 3 passed, 3 total | ||
| Snapshots: 0 total | ||
| Time: 4.375 s, estimated 5 s | ||
| Ran all test suites matching /export-management/i. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex pattern allows paths that don't start with './' or '../' by using
\.\.?\/which makes the second dot optional. This could match invalid paths like./xincorrectly. The pattern should be(\.\.?\/[^'";]+)or more specifically(\.(?:\.\/|\/)[^'";]+)to properly match relative paths.