From 58d064400f32bc66b2f8e72c629276f34786f507 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 23 Sep 2024 21:03:52 +0200 Subject: [PATCH 1/4] Externalize node binary modules for app router --- packages/next/src/build/webpack-config.ts | 13 +++++++++ .../next-error-browser-binary-loader.ts | 11 +++++++ .../loaders/next-server-binary-loader.ts | 14 +++++++++ pnpm-lock.yaml | 4 +-- .../app/layout.tsx | 8 +++++ .../app/page.tsx | 7 +++++ ...ernalize-node-binary-browser-error.test.ts | 29 +++++++++++++++++++ .../foo-browser-import-binary/binary.node | 1 + .../foo-browser-import-binary/index.js | 10 +++++++ .../foo-browser-import-binary/package.json | 4 +++ .../externalize-node-binary/app/layout.tsx | 8 +++++ .../externalize-node-binary/app/page.tsx | 5 ++++ .../externalize-node-binary.test.ts | 15 ++++++++++ .../node_modules/foo/binary.node | 1 + .../node_modules/foo/index.js | 9 ++++++ .../node_modules/foo/package.json | 4 +++ 16 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 packages/next/src/build/webpack/loaders/next-error-browser-binary-loader.ts create mode 100644 packages/next/src/build/webpack/loaders/next-server-binary-loader.ts create mode 100644 test/development/app-dir/externalize-node-binary-browser-error/app/layout.tsx create mode 100644 test/development/app-dir/externalize-node-binary-browser-error/app/page.tsx create mode 100644 test/development/app-dir/externalize-node-binary-browser-error/externalize-node-binary-browser-error.test.ts create mode 100644 test/development/app-dir/externalize-node-binary-browser-error/node_modules/foo-browser-import-binary/binary.node create mode 100644 test/development/app-dir/externalize-node-binary-browser-error/node_modules/foo-browser-import-binary/index.js create mode 100644 test/development/app-dir/externalize-node-binary-browser-error/node_modules/foo-browser-import-binary/package.json create mode 100644 test/e2e/app-dir/externalize-node-binary/app/layout.tsx create mode 100644 test/e2e/app-dir/externalize-node-binary/app/page.tsx create mode 100644 test/e2e/app-dir/externalize-node-binary/externalize-node-binary.test.ts create mode 100644 test/e2e/app-dir/externalize-node-binary/node_modules/foo/binary.node create mode 100644 test/e2e/app-dir/externalize-node-binary/node_modules/foo/index.js create mode 100644 test/e2e/app-dir/externalize-node-binary/node_modules/foo/package.json diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index a4711bc4b9a07..842e39c562432 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -1192,6 +1192,8 @@ export default async function getBaseWebpackConfig( 'next-metadata-route-loader', 'modularize-import-loader', 'next-barrel-loader', + 'next-server-binary-loader', + 'next-error-browser-binary-loader', ].reduce((alias, loader) => { // using multiple aliases to replace `resolveLoader.modules` alias[loader] = path.join(__dirname, 'webpack', 'loaders', loader) @@ -1276,6 +1278,17 @@ export default async function getBaseWebpackConfig( or: WEBPACK_LAYERS.GROUP.nonClientServerTarget, }, }, + { + test: /[\\/].*?\.node$/, + loader: isNodeServer + ? 'next-server-binary-loader' + : 'next-error-browser-binary-loader', + // On server side bundling, only apply to app router, do not apply to pages router; + // On client side or edge runtime bundling, always error. + ...(isNodeServer && { + issuerLayer: isWebpackBundledLayer, + }), + }, ...(hasAppDir ? [ { diff --git a/packages/next/src/build/webpack/loaders/next-error-browser-binary-loader.ts b/packages/next/src/build/webpack/loaders/next-error-browser-binary-loader.ts new file mode 100644 index 0000000000000..36958fa050fa2 --- /dev/null +++ b/packages/next/src/build/webpack/loaders/next-error-browser-binary-loader.ts @@ -0,0 +1,11 @@ +import type { webpack } from 'next/dist/compiled/webpack/webpack' + +export default function nextErrorBrowserBinaryLoader( + this: webpack.LoaderContext +) { + const { resourcePath, rootContext } = this + const relativePath = resourcePath.slice(rootContext.length + 1) + throw new Error( + `Node.js binary module ./${relativePath} is not supported in the browser. Please only use the module on server side` + ) +} diff --git a/packages/next/src/build/webpack/loaders/next-server-binary-loader.ts b/packages/next/src/build/webpack/loaders/next-server-binary-loader.ts new file mode 100644 index 0000000000000..6525734216728 --- /dev/null +++ b/packages/next/src/build/webpack/loaders/next-server-binary-loader.ts @@ -0,0 +1,14 @@ +import type { webpack } from 'next/dist/compiled/webpack/webpack' +import path from 'path' + +export default function nextErrorBrowserBinaryLoader( + this: webpack.LoaderContext +) { + let relativePath = path.relative(this.rootContext, this.resourcePath) + if (!relativePath.startsWith('.')) { + relativePath = './' + relativePath + } + return `module.exports = __non_webpack_require__(${JSON.stringify( + relativePath + )})` +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index da4a63b707573..3aafb357f49d0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -24857,6 +24857,8 @@ packages: dependencies: '@types/unist': 2.0.3 unist-util-stringify-position: 3.0.3 + vfile-sort: 2.2.2 + vfile-statistics: 1.1.4 /vfile-reporter@6.0.2: resolution: {integrity: sha512-GN2bH2gs4eLnw/4jPSgfBjo+XCuvnX9elHICJZjVD4+NM0nsUrMTvdjGY5Sc/XG69XVTgLwj7hknQVc6M9FukA==} @@ -24871,11 +24873,9 @@ packages: /vfile-sort@2.2.2: resolution: {integrity: sha512-tAyUqD2R1l/7Rn7ixdGkhXLD3zsg+XLAeUDUhXearjfIcpL1Hcsj5hHpCoy/gvfK/Ws61+e972fm0F7up7hfYA==} - dev: true /vfile-statistics@1.1.4: resolution: {integrity: sha512-lXhElVO0Rq3frgPvFBwahmed3X03vjPF8OcjKMy8+F1xU/3Q3QU3tKEDp743SFtb74PdF0UWpxPvtOP0GCLheA==} - dev: true /vfile@4.2.1: resolution: {integrity: sha512-O6AE4OskCG5S1emQ/4gl8zK586RqA3srz3nfK/Viy0UPToBc5Trp9BVFb1u0CjsKrAWwnpr4ifM/KBXPWwJbCA==} diff --git a/test/development/app-dir/externalize-node-binary-browser-error/app/layout.tsx b/test/development/app-dir/externalize-node-binary-browser-error/app/layout.tsx new file mode 100644 index 0000000000000..888614deda3ba --- /dev/null +++ b/test/development/app-dir/externalize-node-binary-browser-error/app/layout.tsx @@ -0,0 +1,8 @@ +import { ReactNode } from 'react' +export default function Root({ children }: { children: ReactNode }) { + return ( + + {children} + + ) +} diff --git a/test/development/app-dir/externalize-node-binary-browser-error/app/page.tsx b/test/development/app-dir/externalize-node-binary-browser-error/app/page.tsx new file mode 100644 index 0000000000000..c0acd8c09d702 --- /dev/null +++ b/test/development/app-dir/externalize-node-binary-browser-error/app/page.tsx @@ -0,0 +1,7 @@ +'use client' + +import { foo } from 'foo-browser-import-binary' + +export default function Page() { + return

{foo()}

+} diff --git a/test/development/app-dir/externalize-node-binary-browser-error/externalize-node-binary-browser-error.test.ts b/test/development/app-dir/externalize-node-binary-browser-error/externalize-node-binary-browser-error.test.ts new file mode 100644 index 0000000000000..e67478a937ca7 --- /dev/null +++ b/test/development/app-dir/externalize-node-binary-browser-error/externalize-node-binary-browser-error.test.ts @@ -0,0 +1,29 @@ +import { nextTestSetup } from 'e2e-utils' +import { + assertHasRedbox, + getRedboxDescription, + getRedboxSource, +} from 'next-test-utils' +;(process.env.TURBOPACK ? describe.skip : describe)( + 'externalize-node-binary-browser-error', + () => { + const { next } = nextTestSetup({ + files: __dirname, + }) + + it('should error when import node binary on browser side', async () => { + const browser = await next.browser('/') + await assertHasRedbox(browser) + const redbox = { + description: await getRedboxDescription(browser), + source: await getRedboxSource(browser), + } + + expect(redbox.description).toBe('Failed to compile') + expect(redbox.source).toMatchInlineSnapshot(` + "./node_modules/foo-browser-import-binary/binary.node + Error: Node.js binary module ./node_modules/foo-browser-import-binary/binary.node is not supported in the browser. Please only use the module on server side" + `) + }) + } +) diff --git a/test/development/app-dir/externalize-node-binary-browser-error/node_modules/foo-browser-import-binary/binary.node b/test/development/app-dir/externalize-node-binary-browser-error/node_modules/foo-browser-import-binary/binary.node new file mode 100644 index 0000000000000..c2cdd965f8891 --- /dev/null +++ b/test/development/app-dir/externalize-node-binary-browser-error/node_modules/foo-browser-import-binary/binary.node @@ -0,0 +1 @@ +�This-leading-char-will-trigger-Module parse failed: Unexpected character diff --git a/test/development/app-dir/externalize-node-binary-browser-error/node_modules/foo-browser-import-binary/index.js b/test/development/app-dir/externalize-node-binary-browser-error/node_modules/foo-browser-import-binary/index.js new file mode 100644 index 0000000000000..a91a65011ee59 --- /dev/null +++ b/test/development/app-dir/externalize-node-binary-browser-error/node_modules/foo-browser-import-binary/index.js @@ -0,0 +1,10 @@ +exports.foo = function () { + return 'I am foo' +} + +exports.bar = function () { + if (typeof window !== 'undefined') { + // This will be bundled on server side + require('./binary.node') + } +} diff --git a/test/development/app-dir/externalize-node-binary-browser-error/node_modules/foo-browser-import-binary/package.json b/test/development/app-dir/externalize-node-binary-browser-error/node_modules/foo-browser-import-binary/package.json new file mode 100644 index 0000000000000..ad10d48c9da2e --- /dev/null +++ b/test/development/app-dir/externalize-node-binary-browser-error/node_modules/foo-browser-import-binary/package.json @@ -0,0 +1,4 @@ +{ + "name": "foo-browser-import-binary", + "exports": "./index.js" +} diff --git a/test/e2e/app-dir/externalize-node-binary/app/layout.tsx b/test/e2e/app-dir/externalize-node-binary/app/layout.tsx new file mode 100644 index 0000000000000..888614deda3ba --- /dev/null +++ b/test/e2e/app-dir/externalize-node-binary/app/layout.tsx @@ -0,0 +1,8 @@ +import { ReactNode } from 'react' +export default function Root({ children }: { children: ReactNode }) { + return ( + + {children} + + ) +} diff --git a/test/e2e/app-dir/externalize-node-binary/app/page.tsx b/test/e2e/app-dir/externalize-node-binary/app/page.tsx new file mode 100644 index 0000000000000..bb7deb69423df --- /dev/null +++ b/test/e2e/app-dir/externalize-node-binary/app/page.tsx @@ -0,0 +1,5 @@ +import { foo } from 'foo' + +export default function Page() { + return

{foo()}

+} diff --git a/test/e2e/app-dir/externalize-node-binary/externalize-node-binary.test.ts b/test/e2e/app-dir/externalize-node-binary/externalize-node-binary.test.ts new file mode 100644 index 0000000000000..bf5090cb24ee7 --- /dev/null +++ b/test/e2e/app-dir/externalize-node-binary/externalize-node-binary.test.ts @@ -0,0 +1,15 @@ +import { nextTestSetup } from 'e2e-utils' + +describe('externalize-node-binary', () => { + const { next } = nextTestSetup({ + files: __dirname, + }) + + it('should render correctly when node_modules require node binary module', async () => { + const { status } = await next.fetch('/') + expect(status).toBe(200) + + const browser = await next.browser('/') + expect(await browser.elementByCss('p').text()).toBe('I am foo') + }) +}) diff --git a/test/e2e/app-dir/externalize-node-binary/node_modules/foo/binary.node b/test/e2e/app-dir/externalize-node-binary/node_modules/foo/binary.node new file mode 100644 index 0000000000000..c2cdd965f8891 --- /dev/null +++ b/test/e2e/app-dir/externalize-node-binary/node_modules/foo/binary.node @@ -0,0 +1 @@ +�This-leading-char-will-trigger-Module parse failed: Unexpected character diff --git a/test/e2e/app-dir/externalize-node-binary/node_modules/foo/index.js b/test/e2e/app-dir/externalize-node-binary/node_modules/foo/index.js new file mode 100644 index 0000000000000..e800b36b2de7d --- /dev/null +++ b/test/e2e/app-dir/externalize-node-binary/node_modules/foo/index.js @@ -0,0 +1,9 @@ +exports.foo = function () { + return 'I am foo' +} + +exports.bar = function () { + if (typeof window === 'undefined') { + require('./binary.node') + } +} diff --git a/test/e2e/app-dir/externalize-node-binary/node_modules/foo/package.json b/test/e2e/app-dir/externalize-node-binary/node_modules/foo/package.json new file mode 100644 index 0000000000000..61858e0632858 --- /dev/null +++ b/test/e2e/app-dir/externalize-node-binary/node_modules/foo/package.json @@ -0,0 +1,4 @@ +{ + "name": "foo", + "exports": "./index.js" +} From be7c75a1e5c9a7cba307bba1964d12d60cd671b2 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 30 Sep 2024 23:06:30 +0200 Subject: [PATCH 2/4] fix conflicts --- packages/next/src/build/webpack-config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 842e39c562432..6e84ea45b5fba 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -1286,7 +1286,7 @@ export default async function getBaseWebpackConfig( // On server side bundling, only apply to app router, do not apply to pages router; // On client side or edge runtime bundling, always error. ...(isNodeServer && { - issuerLayer: isWebpackBundledLayer, + issuerLayer: isWebpackAppLayer, }), }, ...(hasAppDir From fee93cab068df0d3830c77c8d20eed6720bcf5d0 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 30 Sep 2024 23:21:29 +0200 Subject: [PATCH 3/4] fix types --- .../externalize-node-binary-browser-error.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/development/app-dir/externalize-node-binary-browser-error/externalize-node-binary-browser-error.test.ts b/test/development/app-dir/externalize-node-binary-browser-error/externalize-node-binary-browser-error.test.ts index e67478a937ca7..2b6ef114bb561 100644 --- a/test/development/app-dir/externalize-node-binary-browser-error/externalize-node-binary-browser-error.test.ts +++ b/test/development/app-dir/externalize-node-binary-browser-error/externalize-node-binary-browser-error.test.ts @@ -1,6 +1,6 @@ import { nextTestSetup } from 'e2e-utils' import { - assertHasRedbox, + hasRedbox, getRedboxDescription, getRedboxSource, } from 'next-test-utils' @@ -13,7 +13,7 @@ import { it('should error when import node binary on browser side', async () => { const browser = await next.browser('/') - await assertHasRedbox(browser) + await hasRedbox(browser) const redbox = { description: await getRedboxDescription(browser), source: await getRedboxSource(browser), From caace1c55d197b79dbbd3c0f209a34d7adb2dd91 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 30 Sep 2024 14:47:50 -0700 Subject: [PATCH 4/4] remove lock change --- pnpm-lock.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3aafb357f49d0..da4a63b707573 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -24857,8 +24857,6 @@ packages: dependencies: '@types/unist': 2.0.3 unist-util-stringify-position: 3.0.3 - vfile-sort: 2.2.2 - vfile-statistics: 1.1.4 /vfile-reporter@6.0.2: resolution: {integrity: sha512-GN2bH2gs4eLnw/4jPSgfBjo+XCuvnX9elHICJZjVD4+NM0nsUrMTvdjGY5Sc/XG69XVTgLwj7hknQVc6M9FukA==} @@ -24873,9 +24871,11 @@ packages: /vfile-sort@2.2.2: resolution: {integrity: sha512-tAyUqD2R1l/7Rn7ixdGkhXLD3zsg+XLAeUDUhXearjfIcpL1Hcsj5hHpCoy/gvfK/Ws61+e972fm0F7up7hfYA==} + dev: true /vfile-statistics@1.1.4: resolution: {integrity: sha512-lXhElVO0Rq3frgPvFBwahmed3X03vjPF8OcjKMy8+F1xU/3Q3QU3tKEDp743SFtb74PdF0UWpxPvtOP0GCLheA==} + dev: true /vfile@4.2.1: resolution: {integrity: sha512-O6AE4OskCG5S1emQ/4gl8zK586RqA3srz3nfK/Viy0UPToBc5Trp9BVFb1u0CjsKrAWwnpr4ifM/KBXPWwJbCA==}