-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Release: Prerelease 8.5.0-alpha.2 #29508
Conversation
In a pnpm monorepo with multiple Next.js projects, different projects can have different peerDependencies for Next.js, which causes pnpm to install multiple instances of the `next` package in the monorepo. In this situation, prior to this change, Storybook's webpack configuration could consume `react` from one instance of Next and `react-dom` from another, causing React render errors due to the mismatch. This issue was caused by the `configureConfig` function adding webpack module aliases for `react` using `addScopedAlias` (which generates an alias with an absolute filesystem path pointing to the correct instance of `next`, e.g. `'/path/to/next/dist/compiled/react'`), but adding module aliases for `react-dom` using `setAlias` (which generates an alias with a relative path, e.g. `'next/dist/compiled/react-dom'`). This would create a webpack configuration like this: ``` alias: { ⋮ react: '/Users/kyank/Developer/authentication-ui/node_modules/.pnpm/[email protected]_@[email protected]_@[email protected]_@[email protected]_babel-plugin_z4uy3ayinaafvek4wmyon66ziu/node_modules/next/dist/compiled/react', ⋮ 'react-dom/test-utils': 'next/dist/compiled/react-dom/cjs/react-dom-test-utils.production.js', 'react-dom$': 'next/dist/compiled/react-dom', 'react-dom/client': 'next/dist/compiled/react-dom/client', 'react-dom/server': 'next/dist/compiled/react-dom/server', ⋮ }, ``` This change uses `addScopedAlias` to create aliases with absolute filesystem paths for all of the Next.js-bundled React packages. This fixes the React rendering errors in our monorepo. This issue seems to have been [introduced in Storybook 8.3.0](https://github.com/storybookjs/storybook/pull/29044/files#diff-20144c44999f6f1054f74f56ef1c3fcfcec008fd7b5caea5e10568e95eccb051).
Build: Gracefully handle empty folders on dev machines
Core: Add bun support with npm fallback
Chore: Improve varous aspects of sandbox creation
…norepo Next.js: Fix bundled react and react-dom in monorepos
Next.js: Upgrade sass-loader from ^13.2.0 to ^14.2.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
17 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings | Greptile
.github/PULL_REQUEST_TEMPLATE.md
Outdated
<!-- BENCHMARK_SECTION --> | ||
<!-- BENCHMARK_SECTION --> No newline at end of file | ||
<!-- BENCHMARK_SECTION --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Duplicate BENCHMARK_SECTION tags - one should be removed
error, | ||
packageManager: 'NPM', | ||
packageName, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Error message shows NPM as package manager even though this is the Bun proxy. Should show Bun instead.
infoCommand: 'npm ls --depth=1', | ||
dedupeCommand: 'npm dedupe', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Info and dedupe commands show npm commands but should reflect Bun equivalents if they exist
@@ -354,6 +355,91 @@ describe('CLASS: JsPackageManagerFactory', () => { | |||
}); | |||
}); | |||
|
|||
describe('Yarn 2 proxy', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: This entire test suite is duplicated from lines 280-356. It should be removed and the Bun test cases should be added as a separate 'Bun proxy' test suite.
@@ -136,6 +146,18 @@ function hasNPM(cwd?: string) { | |||
return npmVersionCommand.status === 0; | |||
} | |||
|
|||
function hasBun(cwd?: string) { | |||
const pnpmVersionCommand = spawnSync('bun', ['--version'], { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Variable name should be 'bunVersionCommand' instead of 'pnpmVersionCommand' for consistency
@@ -22,7 +23,7 @@ export async function setupYarn({ cwd, pnp = false, version = 'classic' }: Setup | |||
|
|||
export async function localizeYarnConfigFiles(baseDir: string, beforeDir: string) { | |||
await Promise.allSettled([ | |||
runCommand(`touch yarn.lock`, { cwd: beforeDir }), | |||
fs.writeFileSync(join(beforeDir, 'yarn.lock'), '', { flag: 'a' }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: writeFileSync is being used in a Promise.allSettled array but it returns void, not a Promise. This could cause unexpected behavior.
d490572
to
cea8510
Compare
…lugin Build: Shim CJS-only globals in ESM output
cea8510
to
5e8aa5e
Compare
…n-prod Addon Test: Only render the TestingModule component in development mode
5e8aa5e
to
01fbde8
Compare
Core: Add support for groups to `TooltipLinkList` and use it for main Storybook menu
01fbde8
to
eea0387
Compare
CLI: Fix Solid init by installing `@storybook/test`
eea0387
to
fe3edcc
Compare
fe3edcc
to
2a0de12
Compare
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 2a0de12. 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 2 targetsSent with 💌 from NxCloud. |
This is an automated pull request that bumps the version from
8.5.0-alpha.1
to8.5.0-alpha.2
.Once this pull request is merged, it will trigger a new release of version
8.5.0-alpha.2
.If you're not a core maintainer with permissions to release you can ignore this pull request.
To do
Before merging the PR, there are a few QA steps to go through:
And for each change below:
This is a list of all the PRs merged and commits pushed directly to
next
, that will be part of this release:@storybook/test
#29514 (will also be patched)TooltipLinkList
and use it in main menu #29507If you've made any changes doing the above QA (change PR titles, revert PRs), manually trigger a re-generation of this PR with this workflow and wait for it to finish. It will wipe your progress in this to do, which is expected.
Feel free to manually commit any changes necessary to this branch after you've done the last re-generation, following the Make Manual Changes section in the docs, especially if you're making changes to the changelog.
When everything above is done:
Generated changelog
8.5.0-alpha.2
@storybook/test
- #29514, thanks @shilman!TooltipLinkList
and use it in main menu - #29507, thanks @ghengeveld!