Test: Consider exports map#32157
Merged
Merged
Conversation
| const parts = path.split('/'); | ||
| // For scoped packages like `@foo/bar`, the package name is the first two parts. | ||
| const packageName = path.startsWith('@') ? `${parts[0]}/${parts[1]}` : parts[0]; | ||
| const entry = `.${path.slice(packageName.length)}`; // e.g., './add' from 'lodash-es/add' |
Contributor
There was a problem hiding this comment.
logic: Entry path calculation will produce incorrect results for root package imports. When path equals packageName (e.g., 'uuid'), path.slice(packageName.length) returns empty string, making entry just '.', but it should be '.' for root imports.
Suggested change
| const entry = `.${path.slice(packageName.length)}`; // e.g., './add' from 'lodash-es/add' | |
| const entry = path === packageName ? '.' : `.${path.slice(packageName.length)}`; // e.g., './add' from 'lodash-es/add' |
|
View your CI Pipeline Execution ↗ for commit 139af72
☁️ Nx Cloud last updated this comment at |
7 tasks
This was referenced Aug 18, 2025
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.
Closes #32151
What I did
Why
This change addresses a critical bug in our module mocking logic that occurred during production builds (
vite buildorwebpack build). The previous implementation usedrequire.resolveto determine the absolute path of mocked node_modules packages. However,require.resolvefollows legacy CJS resolution rules and often fails to respect the "exports" map in a modern package's package.json.This created a mismatch:
uuidto its modern ESM entry point (e.g., .../esm-browser/index.js).Because the absolute paths did not match, our mocking logic failed to identify and replace the module during the build process, causing mocks for packages like uuid to be ignored in the final bundle.
What
This PR updates the resolveMock utility to correctly and reliably resolve modern ESM packages by mimicking the bundler's own resolution strategy.
Prioritizes "exports" Map: The logic now first checks if a package's package.json contains an "exports" map.
Uses resolve.exports: If an "exports" map is present, it uses the
resolve.exportspackage to parse it and find the correct ESM entry point, respecting conditions like "import" and "browser". This guarantees we resolve to the exact same file as Vite and Webpack.Graceful Fallback: If no "exports" map is found, the resolver gracefully falls back to using the classic require.resolve. This ensures continued compatibility with older CJS-style packages.
This change makes our module resolution logic robust and consistent with modern bundler behavior, ensuring that mocks for third-party packages are applied correctly in all environments.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
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.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>Greptile Summary
This PR fixes a critical bug in Storybook's module mocking system that caused third-party package mocks to fail during production builds (
vite buildorwebpack build). The core issue was a resolution mismatch between modern bundlers and Storybook's mocking plugin:uuidto their modern ESM entry points using the "exports" map in package.jsonrequire.resolvewhich follows legacy CJS resolution rules, resolving to different (CJS) entry pointsThis path mismatch prevented the mocking system from identifying and replacing modules during builds, causing mocks to be ignored in production.
The fix updates the
resolveMockutility incode/core/src/core-server/mocking-utils/resolve.tsto:resolve.exports: Parses exports maps with proper ESM conditions ("import", "browser") to match bundler behaviorrequire.resolvefor older CJS packages@scope/packagestyle importsThe changes also include comprehensive test coverage using the
uuidpackage as a test case (which has modern exports maps), mock files, test stories, and sandbox configurations to validate the fix across different environments. The Vite plugin logic was also refined to ensure consistent path matching between development and production builds.Confidence score: 4/5
• This PR addresses a well-defined bug with a targeted solution that maintains backward compatibility
• The core logic changes are sound but involve complex module resolution that could have edge cases
• The
resolve.tsfile needs careful attention as it handles the critical path resolution logic that could break mocking entirely if incorrect