Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Next.js: Add support for Next 15 #29587

Merged
merged 9 commits into from
Nov 12, 2024
Merged

Next.js: Add support for Next 15 #29587

merged 9 commits into from
Nov 12, 2024

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Nov 11, 2024

Closes #29380, #29565, #29582

What I did

This PR introduces support for Next.js 15 (Webpack - @storybook/nextjs) by making changes to the mocks and adding a compatibility layer for Next.js 14. There will be a followup PR for experimental next vite.

It was tested in all sandboxes, as well as in portable stories.

Additionally, it revamps how the Next.js sandboxes are generated:

  • Removes JS sandbox
  • Adds a Next.js v14 sandbox

It also moves all of the Nextjs stories into a single place, so they are all tested in all the nextjs sandboxes. As a result, it also has changes in the e2e specs.

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!

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 canary-release-pr.yml --field pr=<PR_NUMBER>

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 78.2 MB 78.2 MB 0 B 0.88 0%
initSize 143 MB 143 MB 0 B 1.43 0%
diffSize 65.2 MB 65.2 MB 0 B 1.18 0%
buildSize 6.88 MB 6.88 MB 0 B 2.3 0%
buildSbAddonsSize 1.51 MB 1.51 MB 0 B 1 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.9 MB 1.9 MB 0 B 2.34 0%
buildSbPreviewSize 271 kB 271 kB 0 B - 0%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.88 MB 3.88 MB 0 B 2.3 0%
buildPreviewSize 3 MB 3 MB 0 B - 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 9.7s 22.8s 13s 1.11 57.1%
generateTime 21.9s 20.7s -1s -194ms -0.33 -5.8%
initTime 15.4s 15.3s -150ms -0.02 -1%
buildTime 8.5s 8.1s -348ms -0.73 -4.3%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 6.1s 4.9s -1s -192ms -1.53 🔰-24.1%
devManagerResponsive 3.6s 3.1s -564ms -1.55 🔰-18.2%
devManagerHeaderVisible 633ms 489ms -144ms -1.31 🔰-29.4%
devManagerIndexVisible 661ms 558ms -103ms -0.97 -18.5%
devStoryVisibleUncached 1.3s 809ms -491ms -1.25 🔰-60.7%
devStoryVisible 658ms 517ms -141ms -1.35 🔰-27.3%
devAutodocsVisible 603ms 430ms -173ms -1.29 🔰-40.2%
devMDXVisible 675ms 431ms -244ms -1.13 -56.6%
buildManagerHeaderVisible 642ms 477ms -165ms -1.37 🔰-34.6%
buildManagerIndexVisible 675ms 487ms -188ms -1.41 🔰-38.6%
buildStoryVisible 644ms 478ms -166ms -1.33 🔰-34.7%
buildAutodocsVisible 537ms 412ms -125ms -1.04 -30.3%
buildMDXVisible 510ms 402ms -108ms -0.96 -26.9%

Greptile Summary

Added support for Next.js 15 by updating webpack configuration and compatibility mappings to handle breaking changes in Next.js's internal folder structure.

  • Added mapping for next/dist/server/request/headers to next/dist/client/components/headers in code/frameworks/nextjs/src/compatibility/compatibility-map.ts
  • Updated code/frameworks/nextjs/src/export-mocks/headers/index.ts to handle async cookies/headers APIs in Next.js 15
  • Added support for disabling aliases via boolean values in code/frameworks/nextjs/src/export-mocks/webpack.ts
  • Updated peerDependencies in package.json to include Next.js 15 (^15.0.0)
  • Consolidated webpack configuration by moving compatibility and export mock setup into configureAliases

@yannbf yannbf added feature request nextjs ci:merged Run the CI jobs that normally run when merged. labels Nov 11, 2024
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

36 file(s) reviewed, 10 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +21 to +24
(warning) =>
warning.message.includes("export 'draftMode'") &&
warning.message.includes('next/dist/server/request/headers'),
];
Copy link
Contributor

Choose a reason for hiding this comment

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

style: ignoring warnings about missing draftMode export could mask real issues if the export path changes again in future Next.js versions

Comment on lines 39 to 45
Object.entries(aliases).forEach(([name, alias]) => {
addScopedAlias(baseConfig, name, alias);
if (typeof alias === 'string') {
addScopedAlias(baseConfig, name, alias);
} else {
setAlias(baseConfig, name, alias);
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider adding error handling for unexpected alias types

@@ -11,24 +13,26 @@ export default {
src: Accessibility,
alt: 'Accessibility',
},
};
} as Meta<typeof Component>;
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Component is not defined but used in type reference

@@ -10,17 +12,19 @@ export default {
src: Accessibility,
alt: 'Accessibility',
},
};
} as Meta<typeof Component>;
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: 'Component' is undefined, should be 'Image' since that's the component being used in the stories

</p>
);
})}
{(await cookies()).getAll().map(({ name, value }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

style: consider extracting cookie data before mapping to avoid multiple await calls


<h3>Headers:</h3>
{Array.from(headers().entries()).map(([name, value]: [string, string]) => {
{Array.from((await headers()).entries()).map(([name, value]: [string, string]) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

style: consider extracting header entries before mapping to avoid multiple await calls

@@ -10,19 +12,21 @@ export default {
rsc: true,
},
},
};
} as Meta<typeof Component>;
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Component is not defined but used in type annotation. This will cause a TypeScript error. Should be RSC instead of Component

};
} as Meta<typeof Component>;

type Story = StoryObj<typeof Component>;
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Story type uses undefined Component type. Should be type Story = StoryObj<typeof RSC>

@@ -5,7 +5,7 @@ import { cookies } from 'next/headers';
import { redirect } from 'next/navigation';

export async function accessRoute() {
const user = cookies().get('user');
const user = (await cookies()).get('user');
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider adding error handling around the await in case cookies() rejects

@@ -15,6 +15,6 @@ const Component = () => (

export default {
component: Component,
};
} as Meta<typeof Component>;

export const Default = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax: Missing type annotation for Default story export. Should be typed as StoryObj for proper type checking.

@valentinpalkovic
Copy link
Contributor

@yannbf Does this also fix #29582 (comment)?

Copy link

nx-cloud bot commented Nov 12, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 4d3c6d9. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@yannbf
Copy link
Member Author

yannbf commented Nov 12, 2024

@yannbf Does this also fix #29582 (comment)?

Yes it should!

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Only did a cursory review to get this merged. @valentinpalkovic can you please re-review once you have time?

@shilman shilman added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Nov 12, 2024
@shilman shilman changed the title Nextjs: Add support for Next 15 Next.js: Add support for Next 15 Nov 12, 2024
@shilman shilman added maintenance User-facing maintenance tasks and removed feature request labels Nov 12, 2024
@shilman shilman merged commit fa138f6 into next Nov 12, 2024
76 of 80 checks passed
@shilman shilman deleted the yann/next-15-support branch November 12, 2024 09:20
valentinpalkovic pushed a commit that referenced this pull request Nov 12, 2024
Next.js: Add support for Next 15

(cherry picked from commit fa138f6)
yannbf pushed a commit that referenced this pull request Nov 12, 2024
Next.js: Add support for Next 15

(cherry picked from commit fa138f6)
yannbf pushed a commit that referenced this pull request Nov 13, 2024
Next.js: Add support for Next 15

(cherry picked from commit fa138f6)
@github-actions github-actions bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:merged Run the CI jobs that normally run when merged. maintenance User-facing maintenance tasks 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.

[Bug]: Using Storybook with Next@15 RC fails at header mocks
3 participants