Skip to content

Conversation

@yannbf
Copy link
Member

@yannbf yannbf commented Nov 11, 2025

Closes #

What I did

The failure started at v16.0.2-canary.12 where they removed isWasm from the swc index file as can be seen here in favor of using a new utility getBindingsSync. This PR updates the code to be compatible with the new and old way.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

  1. Storybook with Nextjs 15 should work as intented (no changes)
  2. Storybook with Nextjs 16 should work as intended (it was crashing before)

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make 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/core team 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

  • Refactor
    • Internal SWC utility usage reorganized for more consistent and maintainable tooling behavior; runtime behavior and public interfaces unchanged.
  • Style
    • Updated how CSS modules are imported/referenced to standardize styling usage without changing exported APIs.

@yannbf yannbf self-assigned this Nov 11, 2025
@yannbf yannbf added bug patch:yes Bugfix & documentation PR that need to be picked to main branch nextjs ci:daily Run the CI jobs that normally run in the daily job. labels Nov 11, 2025
@nx-cloud
Copy link

nx-cloud bot commented Nov 11, 2025

View your CI Pipeline Execution ↗ for commit c5582fa

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 48s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-11 14:26:57 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 11, 2025

📝 Walkthrough

Walkthrough

Updates Next.js SWC loader patch to use a namespaced nextSwcUtils with async-first WASM detection and nextSwcUtils.transform calls; also changes a React story component to import a CSS module using a namespace import (import * as styles).

Changes

Cohort / File(s) Summary
SWC Loader WASM Detection Refactor
code/frameworks/nextjs/src/swc/next-swc-loader-patch.ts
Replaced direct imports with namespaced nextSwcUtils; pitch() prefers await nextSwcUtils.isWasm() and falls back to nextSwcUtils.getBindingsSync().isWasm after loading bindings; guards now use local isWasm; transform calls updated to nextSwcUtils.transform.
CSS Module Import Style Change
code/renderers/react/template/stories/docgen-components/ts-function-component/input.tsx
Switched CSS module import from default to namespace form: import * as styles from '../imported.module.css';. No logic or export signature changes.

Sequence Diagram(s)

sequenceDiagram
    participant Loader as webpack loader
    participant nextSwcUtils as nextSwcUtils
    participant Bindings as SWC bindings

    rect rgb(245,250,255)
    Loader->>nextSwcUtils: call isWasm() (async)
    alt isWasm() available
        nextSwcUtils-->>Loader: boolean (isWasm)
    else isWasm() not available
        Loader->>Bindings: getBindingsSync()
        Bindings-->>Loader: bindings (with isWasm flag)
    end
    end

    alt isWasm === true
        Loader->>nextSwcUtils: transform(source, options)
        nextSwcUtils-->>Loader: transformed result
    else isWasm === false
        Loader-->>Loader: skip WASM-specific branch / continue regular flow
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify async vs sync WASM detection ordering and potential race conditions in pitch().
  • Confirm nextSwcUtils.transform signature and return shape match previous usage.
  • Check that changing the CSS import to namespace form doesn't break style references in the component.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c5582fa and fc364d7.

📒 Files selected for processing (1)
  • code/renderers/react/template/stories/docgen-components/ts-function-component/input.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (use yarn lint:js:cmd <file>)

Files:

  • code/renderers/react/template/stories/docgen-components/ts-function-component/input.tsx
**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Export functions from modules when they need to be unit-tested

Files:

  • code/renderers/react/template/stories/docgen-components/ts-function-component/input.tsx
code/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

In application code, use Storybook loggers instead of console.* (client code: storybook/internal/client-logger; server code: storybook/internal/node-logger)

Files:

  • code/renderers/react/template/stories/docgen-components/ts-function-component/input.tsx
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/renderers/react/template/stories/docgen-components/ts-function-component/input.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
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.694Z
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.
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mjs} : Export functions from modules when they need to be unit-tested

Applied to files:

  • code/renderers/react/template/stories/docgen-components/ts-function-component/input.tsx
📚 Learning: 2025-11-05T09:38:47.694Z
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.694Z
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/renderers/react/template/stories/docgen-components/ts-function-component/input.tsx
📚 Learning: 2025-11-05T09:37:25.903Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.903Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.

Applied to files:

  • code/renderers/react/template/stories/docgen-components/ts-function-component/input.tsx
⏰ 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). (2)
  • GitHub Check: daily
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/renderers/react/template/stories/docgen-components/ts-function-component/input.tsx (1)

8-8: ****

The namespace import (import * as styles from '../imported.module.css') is the correct pattern. css-loader v7 (April 2024) made a breaking change where named exports became the default when esModule is enabled, which means the default import pattern would break and the fix is to switch to the namespace import.

With this change, styles.foo correctly accesses the named export for the .foo class. The @ts-ignore comment on lines 6–7 indicates this file is intentionally testing edge cases with CSS imports that TypeScript doesn't officially support—this is by design for docgen testing, not an error condition. The code works correctly as written.

Likely an incorrect or invalid review comment.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fca6e7d and 78d459f.

📒 Files selected for processing (1)
  • code/frameworks/nextjs/src/swc/next-swc-loader-patch.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (use yarn lint:js:cmd <file>)

Files:

  • code/frameworks/nextjs/src/swc/next-swc-loader-patch.ts
**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Export functions from modules when they need to be unit-tested

Files:

  • code/frameworks/nextjs/src/swc/next-swc-loader-patch.ts
code/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

In application code, use Storybook loggers instead of console.* (client code: storybook/internal/client-logger; server code: storybook/internal/node-logger)

Files:

  • code/frameworks/nextjs/src/swc/next-swc-loader-patch.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/frameworks/nextjs/src/swc/next-swc-loader-patch.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). (3)
  • GitHub Check: Danger JS
  • GitHub Check: daily
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/frameworks/nextjs/src/swc/next-swc-loader-patch.ts (3)

32-32: LGTM: Namespace import enables runtime API detection.

The namespace import is the correct approach for supporting multiple Next.js versions where different APIs may or may not exist.


139-139: LGTM: Transform call updated consistently with namespace import.


172-172: LGTM: Correctly uses pre-computed WASM detection result.

The conditional now uses the boolean value computed earlier in the function, which is more efficient than calling the detection function multiple times.

Copy link
Member

@ndelangen ndelangen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤪

@yannbf yannbf force-pushed the yann/nextjs-wasm-changes branch from fc364d7 to c5582fa Compare November 11, 2025 14:08
@yannbf yannbf merged commit d12cf1b into next Nov 11, 2025
105 of 109 checks passed
@yannbf yannbf deleted the yann/nextjs-wasm-changes branch November 11, 2025 14:08
yannbf added a commit that referenced this pull request Nov 11, 2025
Next.js: Update SWC loader to support new wasm detection

(cherry picked from commit d12cf1b)
@github-actions github-actions bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Nov 11, 2025
@yannbf yannbf added needs qa Indicates that this needs manual QA during the upcoming minor/major release and removed needs qa Indicates that this needs manual QA during the upcoming minor/major release labels Nov 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ci:daily Run the CI jobs that normally run in the daily job. nextjs patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants