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

Use require.resolve to detect "framework" packages (fix pnpm) #21048

Merged
merged 27 commits into from Nov 9, 2021
Merged
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
3740547
Use require.resolve to detect "framework" packages
elliottsj Jan 13, 2021
f785c27
Add pnpm page-rendering tests
elliottsj Jan 17, 2021
e9c6cc0
Merge remote-tracking branch 'upstream/canary' into split-chunks-fram…
elliottsj Jan 17, 2021
3502549
Merge branch 'canary' into split-chunks-framework-pnpm-fix
elliottsj Jan 25, 2021
a9c2ec8
Merge remote-tracking branch 'upstream/canary' into split-chunks-fram…
elliottsj Jan 26, 2021
c67d0aa
Wait 5 seconds after killing app
elliottsj Jan 26, 2021
0a347ed
Use tree-kill to kill process
elliottsj Jan 28, 2021
edbf543
Merge remote-tracking branch 'upstream/canary' into split-chunks-fram…
elliottsj Jan 28, 2021
c308764
Merge remote-tracking branch 'upstream/canary' into split-chunks-fram…
elliottsj Apr 11, 2021
7e9970d
Merge remote-tracking branch 'upstream/canary' into split-chunks-fram…
elliottsj May 6, 2021
f5594df
Merge remote-tracking branch 'upstream/canary' into split-chunks-fram…
elliottsj Aug 6, 2021
218a400
Use `module.nameForCondition()` to get resource name
elliottsj Aug 10, 2021
8b539e3
Include trailing slash in getPackagePath
elliottsj Aug 10, 2021
fae174b
Merge remote-tracking branch 'upstream/canary' into split-chunks-fram…
elliottsj Aug 10, 2021
f4f0d5e
Merge remote-tracking branch 'upstream/canary' into split-chunks-fram…
elliottsj Aug 15, 2021
3228ceb
Merge remote-tracking branch 'upstream/canary' into split-chunks-fram…
elliottsj Aug 28, 2021
6dd774b
Check for `.nameForCondition` method before calling
elliottsj Aug 28, 2021
4450a12
Remove prop-types from 'framework' chunk paths
elliottsj Aug 28, 2021
41d25a9
Merge remote-tracking branch 'upstream/canary' into split-chunks-fram…
elliottsj Oct 26, 2021
16b2443
Fix pnpm-support tests
elliottsj Oct 29, 2021
5f96288
Merge remote-tracking branch 'upstream/canary' into split-chunks-fram…
elliottsj Oct 29, 2021
a9904ca
Merge branch 'canary' into split-chunks-framework-pnpm-fix
timneutkens Nov 8, 2021
ed1800f
Add object-assign to 'framework' chunk
elliottsj Nov 8, 2021
43dce85
Return execa object in `runPnpm`
elliottsj Nov 8, 2021
758ac4f
Update comment since prop-types is gone
elliottsj Nov 8, 2021
c097bb2
Merge remote-tracking branch 'upstream/canary' into split-chunks-fram…
elliottsj Nov 8, 2021
c25339b
Raise pnpm-support timeout after starting app
elliottsj Nov 9, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 29 additions & 4 deletions packages/next/build/webpack-config.ts
Expand Up @@ -426,6 +426,27 @@ export default async function getBaseWebpackConfig(
)
}

const getPackagePath = (name: string, relativeToPath: string) => {
const packageJsonPath = require.resolve(`${name}/package.json`, {
paths: [relativeToPath],
})
return path.dirname(packageJsonPath)
elliottsj marked this conversation as resolved.
Show resolved Hide resolved
}

// Packages which will be split into the 'framework' chunk.
// Only top-level packages are included, e.g. nested copies like
// 'node_modules/react-scroll-parallax/node_modules/prop-types' are not included.
const topLevelFrameworkPaths = [
elliottsj marked this conversation as resolved.
Show resolved Hide resolved
getPackagePath('react', dir),
getPackagePath('react-dom', dir),
Copy link
Member

Choose a reason for hiding this comment

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

prop-types seem to be missing here?

It was part of the original regexp

Copy link
Member

Choose a reason for hiding this comment

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

Also react and react-dom depend on object-assign so that might also need to be added here, even if it wasn't part of the regexp before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

prop-types should be no longer needed since next no longer declares a direct dep on prop-types. See 4450a12 and c7e2a1d

I'll add object-assign

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

Copy link
Contributor Author

@elliottsj elliottsj Nov 9, 2021

Choose a reason for hiding this comment

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

For what it's worth, this error is raised when getPackagePath('prop-types', dir) is added to the array:

> pnpm-app@1.0.0 build /private/var/folders/zj/25m6hvw14q5fgtm87smrxsbc0000gn/T/hobwb9bsxk9/app
> next build

info  - Checking validity of types
warn  - No ESLint configuration detected. Run next lint to begin setup
info  - Disabled SWC as replacement for Babel because of custom Babel configuration ".babelrc" https://nextjs.org/docs/messages/swc-disabled

> Build error occurred
Error: Cannot find module 'prop-types/package.json'
Require stack:
- /private/var/folders/zj/25m6hvw14q5fgtm87smrxsbc0000gn/T/hobwb9bsxk9/app/node_modules/.pnpm/local++var+folders+zj+25m6hvw14q5fgtm87smrxsbc0000_05da36a7aef9195a59c6f2b92d013fed/node_modules/next/dist/build/webpack-config.js
- /private/var/folders/zj/25m6hvw14q5fgtm87smrxsbc0000gn/T/hobwb9bsxk9/app/node_modules/.pnpm/local++var+folders+zj+25m6hvw14q5fgtm87smrxsbc0000_05da36a7aef9195a59c6f2b92d013fed/node_modules/next/dist/build/index.js

getPackagePath('scheduler', require.resolve('react-dom', { paths: [dir] })),
getPackagePath('prop-types', require.resolve('next', { paths: [dir] })),
getPackagePath(
'use-subscription',
require.resolve('next', { paths: [dir] })
),
]

// Contains various versions of the Webpack SplitChunksPlugin used in different build types
const splitChunksConfigs: {
[propName: string]: webpack.Options.SplitChunksOptions
Expand All @@ -448,10 +469,14 @@ export default async function getBaseWebpackConfig(
framework: {
chunks: 'all',
name: 'framework',
// This regex ignores nested copies of framework libraries so they're
// bundled with their issuer.
// https://github.com/vercel/next.js/pull/9012
test: /(?<!node_modules.*)[\\/]node_modules[\\/](react|react-dom|scheduler|prop-types|use-subscription)[\\/]/,
test(module) {
if (!module.resource) {
return false
}
return topLevelFrameworkPaths.some((packagePath) =>
module.resource.startsWith(packagePath)
)
},
elliottsj marked this conversation as resolved.
Show resolved Hide resolved
priority: 40,
// Don't let webpack eliminate this chunk (prevents this chunk from
// becoming a part of the commons chunk)
Expand Down