Skip to content

Conversation

@ndelangen
Copy link
Member

@ndelangen ndelangen commented Sep 29, 2025

Reverts #32547

Seems there may have been some unintentional UI changes due to this, checking if reverting this PR fixes it, if not something else was the cause.

Summary by CodeRabbit

  • Breaking Changes

    • Removed deprecated preview export for Next.js and Next.js Vite frameworks. Switch to the default preview entry.
  • Improvements

    • Unified preview behavior across all Next.js versions; no version-specific setup required.
    • More reliable initialization and runtime configuration in preview.
  • Refactor

    • Simplified framework configuration by removing version checks and related dependencies, reducing surface area and maintenance overhead.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 29, 2025

Walkthrough

This PR removes the exported entry "./config/preview" from both nextjs and nextjs-vite packages (build-config and package.json). It deletes Next.js version detection logic by removing semver usage and the getNextjsVersion utility. Presets now always include the preview entry ('@storybook/nextjs' or '@storybook/nextjs-vite/preview') without version checks. preview.tsx adds a local side-effect import of ./config/preview. In Next.js webpack config, aliasing for 'next/config' and DefinePlugin runtime config setup are now unconditional.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant SB as Storybook
  participant Preset as Preset (nextjs / nextjs-vite)
  participant Preview as preview.tsx
  participant Cfg as ./config/preview
  note over Preset: Version checks removed

  SB->>Preset: resolve previewAnnotations()
  Preset-->>SB: [..., '@storybook/nextjs(-vite)/preview']

  SB->>Preview: load preview.tsx
  Preview->>Cfg: import for side effects
  Cfg-->>Preview: initialize preview config
  Preview-->>SB: decorators/parameters ready
Loading
sequenceDiagram
  autonumber
  participant Builder as Builder
  participant WConf as Next.js webpack config
  participant WP as Webpack

  Builder->>WConf: getConfig()
  note over WConf: Unconditional setup
  WConf->>WP: add alias('next/config')
  WConf->>WP: DefinePlugin({ 'process.env.__NEXT_RUNTIME_CONFIG': JSON.stringify(...) })
  WConf-->>Builder: final webpack config
Loading

Possibly related PRs

Suggested labels

maintenance, nextjs, ci:normal

Suggested reviewers

  • valentinpalkovic

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly indicates that this pull request reverts the earlier change removing next/config usage in Next.js projects version 16 and above, which aligns directly with the stated objective of rolling back the unintended UI changes from the original removal.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch revert-32547-valentin/drop-support-for-next-config-in-Next-v16

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

@github-actions
Copy link
Contributor

Fails
🚫

PR is not labeled with one of: ["cleanup","BREAKING CHANGE","feature request","bug","documentation","maintenance","build","dependencies"]

🚫

PR is not labeled with one of: ["ci:normal","ci:merged","ci:daily","ci:docs"]

Generated by 🚫 dangerJS against c68a102

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: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df23220 and c68a102.

⛔ Files ignored due to path filters (1)
  • code/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (10)
  • code/frameworks/nextjs-vite/build-config.ts (0 hunks)
  • code/frameworks/nextjs-vite/package.json (0 hunks)
  • code/frameworks/nextjs-vite/src/preset.ts (1 hunks)
  • code/frameworks/nextjs-vite/src/preview.tsx (2 hunks)
  • code/frameworks/nextjs-vite/src/utils.ts (0 hunks)
  • code/frameworks/nextjs/build-config.ts (0 hunks)
  • code/frameworks/nextjs/package.json (0 hunks)
  • code/frameworks/nextjs/src/config/webpack.ts (3 hunks)
  • code/frameworks/nextjs/src/preset.ts (1 hunks)
  • code/frameworks/nextjs/src/preview.tsx (1 hunks)
💤 Files with no reviewable changes (5)
  • code/frameworks/nextjs/package.json
  • code/frameworks/nextjs-vite/src/utils.ts
  • code/frameworks/nextjs/build-config.ts
  • code/frameworks/nextjs-vite/build-config.ts
  • code/frameworks/nextjs-vite/package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}

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

Adhere to ESLint and Prettier rules across all JS/TS source files

Files:

  • code/frameworks/nextjs-vite/src/preset.ts
  • code/frameworks/nextjs-vite/src/preview.tsx
  • code/frameworks/nextjs/src/preset.ts
  • code/frameworks/nextjs/src/preview.tsx
  • code/frameworks/nextjs/src/config/webpack.ts
**/*.{ts,tsx}

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

Fix type errors and prefer precise typings instead of using any or suppressions, consistent with strict mode

Files:

  • code/frameworks/nextjs-vite/src/preset.ts
  • code/frameworks/nextjs-vite/src/preview.tsx
  • code/frameworks/nextjs/src/preset.ts
  • code/frameworks/nextjs/src/preview.tsx
  • code/frameworks/nextjs/src/config/webpack.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: ndelangen
PR: storybookjs/storybook#32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.209Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
📚 Learning: 2025-09-24T09:39:39.209Z
Learnt from: ndelangen
PR: storybookjs/storybook#32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.209Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.

Applied to files:

  • code/frameworks/nextjs-vite/src/preview.tsx
  • code/frameworks/nextjs/src/preview.tsx
🧬 Code graph analysis (1)
code/frameworks/nextjs/src/config/webpack.ts (1)
code/frameworks/nextjs/src/utils.ts (1)
  • addScopedAlias (71-75)
⏰ 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). (1)
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (4)
code/frameworks/nextjs-vite/src/preview.tsx (1)

16-16: Thanks for restoring the preview side-effect import

Re-adding ./config/preview ensures the preset-level preview hooks run again before decorators execute—exactly what we want after the revert.

code/frameworks/nextjs/src/preview.tsx (1)

17-17: Preview side-effect restored correctly

Re-introducing the local ./config/preview import brings back the side effects that initialize the Next-specific preview shims before the decorators run. Looks good to me.

code/frameworks/nextjs/src/preset.ts (1)

50-52: Preview entry inclusion reverted properly

Appending @storybook/nextjs/preview back into the annotations list restores the previous preload ordering so the preview config runs as expected. No concerns here.

code/frameworks/nextjs/src/config/webpack.ts (1)

6-76: Next runtime config shim reinstated

Bringing back the addScopedAlias('next/config') plus the DefinePlugin payload for __NEXT_RUNTIME_CONFIG re-aligns the webpack setup with the pre-change behavior, so consumers depending on the shim will work again. Looks solid.

Comment on lines +38 to +39
const result = [...entry, fileURLToPath(import.meta.resolve('@storybook/nextjs-vite/preview'))];
return result;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Revert missed: preview entry now resolves to a non-existent export

The revert restores the ./config/preview export and removes the ./preview entry that #32547 introduced. Keeping fileURLToPath(import.meta.resolve('@storybook/nextjs-vite/preview')) means this will resolve to a module that no longer exists and Storybook will crash with ERR_MODULE_NOT_FOUND as soon as preview annotations are computed. Please switch this back to the @storybook/nextjs-vite/config/preview path (and reapply the old version-gating logic if needed) so the revert actually works.

🤖 Prompt for AI Agents
In code/frameworks/nextjs-vite/src/preset.ts around lines 38-39, the preview
entry currently resolves '@storybook/nextjs-vite/preview' which no longer
exists; revert this to the original '@storybook/nextjs-vite/config/preview' (and
if the prior version-gating check existed, reapply it) so Storybook loads the
restored ./config/preview export instead of a non-existent ./preview module;
update the resolved path accordingly and ensure any conditional logic that
selected between legacy and new preview paths is restored.

import type { Addon_DecoratorFunction, LoaderFunction } from 'storybook/internal/types';

import type { ReactRenderer } from '@storybook/react';
import type { ReactRenderer, StoryFn } from '@storybook/react';
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Remove the unused StoryFn import

StoryFn isn’t referenced anywhere in this file, so the TS/ESLint “no-unused-locals” rule will fail the build. Please drop it from the import list (or wire it back into the typing if that was the intention).

-import type { ReactRenderer, StoryFn } from '@storybook/react';
+import type { ReactRenderer } from '@storybook/react';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import type { ReactRenderer, StoryFn } from '@storybook/react';
// Drop the unused `StoryFn` import
import type { ReactRenderer } from '@storybook/react';
import './config/preview';
🤖 Prompt for AI Agents
In code/frameworks/nextjs-vite/src/preview.tsx around line 5, the imported
symbol StoryFn is not used which will trigger the no-unused-locals rule; remove
StoryFn from the import list (i.e. import only ReactRenderer) or, if you
intended to use it, add the appropriate type annotation where StoryFn is needed
so the import is referenced.

@nx-cloud
Copy link

nx-cloud bot commented Sep 29, 2025

View your CI Pipeline Execution ↗ for commit c68a102

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

☁️ Nx Cloud last updated this comment at 2025-09-29 08:03:15 UTC

@ndelangen ndelangen closed this Sep 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants