From 096fb495bd7d1d2f615d32620907499e1b82e919 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 23 Oct 2024 15:55:44 +0200 Subject: [PATCH 1/3] Fix: revert the bad node binary handling --- packages/next/src/build/webpack-config.ts | 19 ++++++++----------- .../loaders/next-server-binary-loader.ts | 1 + .../externalize-node-binary.test.ts | 3 ++- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 57ab6cee13f39..b8555dec392fb 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -1291,17 +1291,14 @@ export default async function getBaseWebpackConfig( or: WEBPACK_LAYERS.GROUP.neutralTarget, }, }, - { - 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, - }), - }, + ...(isNodeServer + ? [] + : [ + { + test: /[\\/].*?\.node$/, + loader: 'next-error-browser-binary-loader', + }, + ]), ...(hasAppDir ? [ { 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 index a395d8476fd7d..030187da434c8 100644 --- a/packages/next/src/build/webpack/loaders/next-server-binary-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-server-binary-loader.ts @@ -8,5 +8,6 @@ export default function nextErrorBrowserBinaryLoader( if (!relativePath.startsWith('.')) { relativePath = './' + relativePath } + // FIXME: the path resolving is not robust return `module.exports = __non_webpack_require__(${JSON.stringify(relativePath)})` } 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 index bf5090cb24ee7..07017d378bd4e 100644 --- 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 @@ -1,6 +1,7 @@ import { nextTestSetup } from 'e2e-utils' -describe('externalize-node-binary', () => { +// FIXME: er-enable when we have a better implementation of node binary resolving +describe.skip('externalize-node-binary', () => { const { next } = nextTestSetup({ files: __dirname, }) From 9e70e92c08c68d3c28adda2a75dae8050e42366a Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 23 Oct 2024 16:06:39 +0200 Subject: [PATCH 2/3] disable tests --- ...ernalize-node-binary-browser-error.test.ts | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 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..8dd9d5e51b541 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 @@ -4,26 +4,25 @@ import { 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), - } +// FIXME: er-enable when we have a better implementation of node binary resolving +describe.skip('externalize-node-binary-browser-error', () => { + const { next } = nextTestSetup({ + files: __dirname, + }) - expect(redbox.description).toBe('Failed to compile') - expect(redbox.source).toMatchInlineSnapshot(` + 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" `) - }) - } -) + }) +}) From 665fa13816ce828446c92881d778b84ce1c69333 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 23 Oct 2024 16:21:03 +0200 Subject: [PATCH 3/3] rm loader --- packages/next/src/build/webpack-config.ts | 1 - .../webpack/loaders/next-server-binary-loader.ts | 13 ------------- 2 files changed, 14 deletions(-) delete mode 100644 packages/next/src/build/webpack/loaders/next-server-binary-loader.ts diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index b8555dec392fb..d65810e6c31e9 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -1202,7 +1202,6 @@ 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) => { 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 deleted file mode 100644 index 030187da434c8..0000000000000 --- a/packages/next/src/build/webpack/loaders/next-server-binary-loader.ts +++ /dev/null @@ -1,13 +0,0 @@ -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 - } - // FIXME: the path resolving is not robust - return `module.exports = __non_webpack_require__(${JSON.stringify(relativePath)})` -}