Maintenance: Centralize supported file extension lists#34844
Maintenance: Centralize supported file extension lists#34844valentinpalkovic wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughCentralizes multiple hardcoded file-extension lists into a new shared constants module and updates module resolution, Storybook config detection, import parsing, and mock resolution to use those shared extension arrays. ChangesExtension Consolidation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
Address @Sidnioulz's review feedback (PR #34692). Inline extension lists were scattered across code/core with subtle drift between similar tasks. Introduces `code/core/src/shared/constants/extensions.ts` with four task-named constants: - `jsModuleExtensions` — .mjs/.js/.cjs only - `jsTsSourceExtensions` — JS/TS family incl. JSX/TSX - `storybookConfigExtensions` — main/preview/manager config files - `userModuleExtensions` — user code incl. JSON, .vue, .svelte Migrates five consumers: - common/utils/interpret-files.ts → storybookConfigExtensions (keeps the public `supportedExtensions` re-export used by the react renderer's component manifest) - mocking-utils/resolve.ts → userModuleExtensions - common/utils/validate-config.ts → jsModuleExtensions - shared/utils/module.ts → jsModuleExtensions - core-server/change-detection/parser-registry/builtins.ts → jsTsSourceExtensions Left intentionally inline (different task / subtle behavior): - bin/loader.ts — runtime ESM/CJS loader, narrower set - builder-manager/index.ts — esbuild bundler config - telemetry/get-application-file-count.ts — git ls-files glob (no dots) No behavioral changes; pure deduplication. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
e3d9256 to
f692fe6
Compare
| * Module file extensions for user-code resolution (e.g. `sb.mock()`). Covers JS/TS plus JSON and | ||
| * common single-file-component types (`.vue`, `.svelte`). | ||
| */ | ||
| export const userModuleExtensions = [ | ||
| '.js', | ||
| '.mjs', | ||
| '.cjs', | ||
| '.ts', | ||
| '.tsx', | ||
| '.jsx', | ||
| '.json', | ||
| '.vue', | ||
| '.svelte', |
There was a problem hiding this comment.
Thanks so much for that!
WDYT about my suggestion on the other PR to also add e.g. Solid or Astro? Like Vue and Svelte, these are supported when the relevant framework is used.
There was a problem hiding this comment.
Has Solid its own file extension and DSL? I'm okay with adding astro for sure!
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 184 | 184 | 0 |
| Self size | 79 KB | 79 KB | 0 B |
| Dependency size | 33.35 MB | 33.36 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 160 KB | 160 KB | 🎉 -54 B 🎉 |
| Dependency size | 30.73 MB | 30.75 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 188 | 188 | 0 |
| Self size | 15 KB | 15 KB | 🚨 +18 B 🚨 |
| Dependency size | 30.07 MB | 30.08 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 534 | 534 | 0 |
| Self size | 662 KB | 662 KB | 🎉 -120 B 🎉 |
| Dependency size | 61.51 MB | 61.52 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 271 | 271 | 0 |
| Self size | 23 KB | 23 KB | 🎉 -12 B 🎉 |
| Dependency size | 46.05 MB | 46.06 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 196 | 196 | 0 |
| Self size | 16 KB | 16 KB | 0 B |
| Dependency size | 34.61 MB | 34.63 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 164 | 164 | 0 |
| Self size | 18 KB | 18 KB | 0 B |
| Dependency size | 32.35 MB | 32.37 MB | 🚨 +16 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
What I did
Follow-up to #34692. Addresses @Sidnioulz's review feedback that the codebase had several inline extension lists with subtle drift between similar tasks.
Introduces
code/core/src/shared/constants/extensions.tswith four task-named constants:jsModuleExtensions.mjs .js .cjsjsTsSourceExtensions.ts .tsx .js .jsx .mjs .cjsstorybookConfigExtensionsmain,preview,manager).js .ts .jsx .tsx .mjs .mts .mtsx .cjs .cts .ctsxuserModuleExtensionssb.mock()(incl. JSON, SFC).js .mjs .cjs .ts .tsx .jsx .json .vue .svelteMigrates five consumers to import from the central module:
common/utils/interpret-files.ts→storybookConfigExtensions(keeps the publicsupportedExtensionsre-export used by the React renderer's component manifest)mocking-utils/resolve.ts→userModuleExtensionscommon/utils/validate-config.ts→jsModuleExtensionsshared/utils/module.ts→jsModuleExtensionscore-server/change-detection/parser-registry/builtins.ts→jsTsSourceExtensionsThree lists were intentionally left inline because they serve different tasks with subtle behavioral coupling:
bin/loader.ts— runtime ESM/CJS hook loader uses a narrower set (no.mtsx/.ctsx)builder-manager/index.ts— esbuild bundler config, changing it would alter what esbuild resolves in the manager bundletelemetry/get-application-file-count.ts— git ls-files glob using a no-dot formatNo behavioral changes; pure deduplication.
Why now
Reviewer-requested follow-up to #34692 (resolve → oxc-resolver migration). Doing it as a separate PR keeps the dependency migration focused and the refactor reviewable on its own.
Stacking
This PR is stacked on top of #34692. Once #34692 merges, this auto-rebases against
next.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Existing tests cover all five migrated consumers. The refactor is a pure substitution (each new constant equals the inline list it replaces, byte-for-byte).
Manual testing
yarn nx check coreandyarn nx check reactboth pass. Lint clean on all touched files.Documentation
Checklist for Maintainers
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tscleanupormaintenance.🤖 Generated with Claude Code
Summary by CodeRabbit