Webpack: Revert "disable bugfixes property in swc and babel#33498
Conversation
…pport the alternatively destructured syntax in our play-function parsing/detection of mount" This reverts commit 7311877.
ndelangen
added
bug
api: stories
nextjs
builder-webpack5
ci:merged
|
View your CI Pipeline Execution ↗ for commit f047773
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit f047773 ☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughThe pull request enables the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @code/frameworks/nextjs/src/swc/next-swc-loader-patch.ts:
- Around line 116-119: Ensure programmaticOptions.env exists before setting
bugfixes: when you see the direct assignment to programmaticOptions.env.bugfixes
change it to first ensure programmaticOptions.env is an object (create an empty
object if undefined) and then set programmaticOptions.env.bugfixes = true (or
only set it when undefined if you prefer to preserve an explicit existing
value); update the code around the programmaticOptions.env.bugfixes assignment
to follow this defensive pattern.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
code/builders/builder-webpack5/src/presets/custom-webpack-preset.tscode/core/src/core-server/presets/common-preset.tscode/frameworks/nextjs/src/babel/preset.tscode/frameworks/nextjs/src/preset.tscode/frameworks/nextjs/src/swc/next-swc-loader-patch.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/builders/builder-webpack5/src/presets/custom-webpack-preset.tscode/frameworks/nextjs/src/swc/next-swc-loader-patch.tscode/frameworks/nextjs/src/preset.tscode/frameworks/nextjs/src/babel/preset.tscode/core/src/core-server/presets/common-preset.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/builders/builder-webpack5/src/presets/custom-webpack-preset.tscode/frameworks/nextjs/src/swc/next-swc-loader-patch.tscode/frameworks/nextjs/src/preset.tscode/frameworks/nextjs/src/babel/preset.tscode/core/src/core-server/presets/common-preset.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/builders/builder-webpack5/src/presets/custom-webpack-preset.tscode/frameworks/nextjs/src/swc/next-swc-loader-patch.tscode/frameworks/nextjs/src/preset.tscode/frameworks/nextjs/src/babel/preset.tscode/core/src/core-server/presets/common-preset.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/builders/builder-webpack5/src/presets/custom-webpack-preset.tscode/frameworks/nextjs/src/swc/next-swc-loader-patch.tscode/frameworks/nextjs/src/preset.tscode/frameworks/nextjs/src/babel/preset.tscode/core/src/core-server/presets/common-preset.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/builders/builder-webpack5/src/presets/custom-webpack-preset.tscode/frameworks/nextjs/src/swc/next-swc-loader-patch.tscode/frameworks/nextjs/src/preset.tscode/frameworks/nextjs/src/babel/preset.tscode/core/src/core-server/presets/common-preset.ts
🧠 Learnings (2)
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/frameworks/nextjs/src/preset.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Use `NODE_OPTIONS=--max_old_space_size=4096` for memory optimization when running tests
Applied to files:
code/frameworks/nextjs/src/babel/preset.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: merged
- GitHub Check: nx
- GitHub Check: nx
- GitHub Check: nx
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (4)
code/frameworks/nextjs/src/babel/preset.ts (1)
95-95: LGTM! Correctly enables bugfixes for Babel preset-env.The
bugfixes: trueoption is properly configured in the preset-env configuration, aligning with the PR objective to prevent unnecessary transpilation of Safari-compatible syntax.code/frameworks/nextjs/src/preset.ts (1)
129-129: LGTM! Consistent with core preset configuration.The
bugfixes: trueoption correctly mirrors the configuration incode/core/src/core-server/presets/common-preset.tsfor story file overrides, ensuring consistent transpilation behavior across the Next.js framework integration.code/builders/builder-webpack5/src/presets/custom-webpack-preset.ts (1)
26-29: LGTM! Excellent defensive coding pattern.The implementation uses nullish coalescing (
config?.env?.bugfixes ?? true) to safely default totruewhile respecting user configuration. Theenvobject is properly constructed with the spread operator fallback (...(config?.env ?? {})), preventing potential runtime errors.code/core/src/core-server/presets/common-preset.ts (1)
123-123: LGTM! Canonical configuration for story file transpilation.This is the reference Babel configuration that prevents over-transpilation for mount context detection in play functions. The
bugfixes: trueoption is correctly applied to story files, and other framework presets (like Next.js) mirror this configuration.
| // Transpiles the broken syntax to the closest non-broken modern syntax. | ||
| // E.g. it won't transpile parameter destructuring in Safari | ||
| // which would break how we detect if the mount context property is used in the play function. | ||
| programmaticOptions.env.bugfixes = true; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file in question
cat -n code/frameworks/nextjs/src/swc/next-swc-loader-patch.ts | head -150Repository: storybookjs/storybook
Length of output: 6244
🏁 Script executed:
# Search for getLoaderSWCOptions function definition
rg -nP -A 30 'export (const|function) getLoaderSWCOptions' --type=ts --type=jsRepository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
# Also check for any type definitions or interfaces for programmaticOptions
rg -nP 'interface.*SWCOptions|type.*SWCOptions|programmaticOptions.*=' --type=ts --type=js code/frameworks/nextjs/src/swc/Repository: storybookjs/storybook
Length of output: 547
🏁 Script executed:
# Check the Next.js version in dependencies
rg '"next"' code/frameworks/nextjs/package.jsonRepository: storybookjs/storybook
Length of output: 144
🏁 Script executed:
# Search for any SWC options type definitions or documentation in the repo
rg -A 10 'env.*bugfixes|env:.*{' --type=ts --type=js code/frameworks/nextjs/Repository: storybookjs/storybook
Length of output: 170
🏁 Script executed:
# Look for other places where getLoaderSWCOptions is used or where env is accessed on SWC options
rg 'getLoaderSWCOptions|\.env\.' --type=ts --type=js code/frameworks/nextjs/ -B 2 -A 2Repository: storybookjs/storybook
Length of output: 4659
🏁 Script executed:
# Check if there's a Next.js patch or version check
cat code/frameworks/nextjs/package.jsonRepository: storybookjs/storybook
Length of output: 4745
🏁 Script executed:
# Search for tests related to next-swc-loader-patch
fd -e test.ts -e test.js -e spec.ts -e spec.js -e .test.ts code/frameworks/nextjs/ | head -20Repository: storybookjs/storybook
Length of output: 462
🏁 Script executed:
# Search for any usage/reference to swcOptions or env in the context of SWC
rg -A 5 -B 5 'swcOptions' code/frameworks/nextjs/src/ --type=ts --type=jsRepository: storybookjs/storybook
Length of output: 1879
🏁 Script executed:
# Check if there are any comments or documentation about the structure
rg -A 10 'getLoaderSWCOptions' code/frameworks/nextjs/ --type=ts --type=jsRepository: storybookjs/storybook
Length of output: 1956
🏁 Script executed:
# Look for version checks or compatibility handling
rg 'getNextjsVersion|version' code/frameworks/nextjs/src/utils.ts -A 5 -B 5Repository: storybookjs/storybook
Length of output: 1440
🏁 Script executed:
# Search for test files in the nextjs framework
fd . code/frameworks/nextjs --type f -name "*.test.*" -o -name "*.spec.*"Repository: storybookjs/storybook
Length of output: 295
🏁 Script executed:
# Look for any imports or type definitions related to swc in the repo
rg 'type.*SWC|interface.*SWC' code/frameworks/nextjs --type=tsRepository: storybookjs/storybook
Length of output: 156
🏁 Script executed:
# Check if there are any comments or patterns in how swcOptions is used in other parts of the codebase
rg -B 10 -A 10 'programmaticOptions\.env|swcOptions\.env' --type=ts --type=jsRepository: storybookjs/storybook
Length of output: 1988
🏁 Script executed:
# Search in broader Storybook codebase for SWC patterns
rg 'env.*bugfixes|bugfixes.*true' --type=ts --type=jsRepository: storybookjs/storybook
Length of output: 489
🏁 Script executed:
# Get more context around the builder-webpack5 pattern
rg -B 15 -A 5 'bugfixes: config' code/builders/builder-webpack5/src/presets/custom-webpack-preset.tsRepository: storybookjs/storybook
Length of output: 807
🏁 Script executed:
# Check the file that was mentioned in the original review as context
cat -n code/builders/builder-webpack5/src/presets/custom-webpack-preset.ts | sed -n '1,50p'Repository: storybookjs/storybook
Length of output: 2216
🏁 Script executed:
# Search for any historical issues or comments about env property
rg -i 'env.*property|env.*undefined|env.*null' code/frameworks/nextjs/ --type=ts --type=jsRepository: storybookjs/storybook
Length of output: 47
Verify that programmaticOptions.env exists before assignment.
The code directly assigns programmaticOptions.env.bugfixes = true without verifying that the env property exists. If getLoaderSWCOptions returns an object without an env property, this will throw a TypeError.
Compare this to the defensive pattern used in the same codebase (code/builders/builder-webpack5/src/presets/custom-webpack-preset.ts), which uses identical logic but with a defensive approach:
env: {
...(config?.env ?? {}),
bugfixes: config?.env?.bugfixes ?? true,
},Additionally, the code already demonstrates defensive checking patterns elsewhere (lines 128–133 verify programmaticOptions.jsc before nested access). Apply the same defensive approach here:
programmaticOptions.env = programmaticOptions.env || {};
programmaticOptions.env.bugfixes = true;🤖 Prompt for AI Agents
In @code/frameworks/nextjs/src/swc/next-swc-loader-patch.ts around lines 116 -
119, Ensure programmaticOptions.env exists before setting bugfixes: when you see
the direct assignment to programmaticOptions.env.bugfixes change it to first
ensure programmaticOptions.env is an object (create an empty object if
undefined) and then set programmaticOptions.env.bugfixes = true (or only set it
when undefined if you prefer to preserve an explicit existing value); update the
code around the programmaticOptions.env.bugfixes assignment to follow this
defensive pattern.
1 participant
This reverts commit 7311877.
What I did
I'm reverting this change because:
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
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 publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.