From 60dd6dfc9bf2c2b04965d09ed87d1be591986ad0 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 13 Dec 2022 18:16:01 +0100 Subject: [PATCH] Fix module error for findDOMNode on edge (#43998) ## Bug The `findDOMNode` will be exported from ReactDOM in esm mode, but it's not defined for SSR since SSR is using react-dom server stub bundle which doesn't contain any thing. So `import { findDOMNode } from 'react-dom'` will error in that case with bundling. Since it's only being used on client, we import ReactDOM and call `ReactDOM.findDOMNode` to avoid bundling error and adding a condition to tree-shake it off on client [slack thread](https://vercel.slack.com/archives/C03KAR5DCKC/p1670608621289259) - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) --- .../next/client/components/layout-router.tsx | 14 ++++++++------ test/e2e/app-dir/app-edge.test.ts | 16 +++++++++++++--- test/e2e/streaming-ssr/index.test.ts | 8 -------- .../streaming-ssr/streaming-ssr/pages/router.js | 4 +--- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index 670de2e7b840e65..aa482626e8780c9 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -9,10 +9,10 @@ import type { } from '../../server/app-render' import type { ErrorComponent } from './error-boundary' import type { FocusAndScrollRef } from './reducer' +import type { ChildProp } from '../../server/app-render' import React, { useContext, useEffect, use } from 'react' -import { findDOMNode as ReactDOMfindDOMNode } from 'react-dom' -import type { ChildProp } from '../../server/app-render' +import ReactDOM from 'react-dom' import { CacheStates, LayoutRouterContext, @@ -80,8 +80,10 @@ function walkAddRefetch( * Wraps ReactDOM.findDOMNode with additional logic to hide React Strict Mode warning */ function findDOMNode( - instance: Parameters[0] -): ReturnType { + instance: Parameters[0] +): ReturnType { + // Tree-shake for server bundle + if (typeof window === undefined) return null // Only apply strict mode warning when not in production if (process.env.NODE_ENV !== 'production') { const originalConsoleError = console.error @@ -92,12 +94,12 @@ function findDOMNode( originalConsoleError(...messages) } } - return ReactDOMfindDOMNode(instance) + return ReactDOM.findDOMNode(instance) } finally { console.error = originalConsoleError! } } - return ReactDOMfindDOMNode(instance) + return ReactDOM.findDOMNode(instance) } /** diff --git a/test/e2e/app-dir/app-edge.test.ts b/test/e2e/app-dir/app-edge.test.ts index 25873b92fa38a27..f5d4dc2591c08e6 100644 --- a/test/e2e/app-dir/app-edge.test.ts +++ b/test/e2e/app-dir/app-edge.test.ts @@ -34,6 +34,17 @@ describe('app-dir edge SSR', () => { }) if ((globalThis as any).isNextDev) { + it('should resolve module without error in edge runtime', async () => { + const logs = [] + next.on('stderr', (log) => { + logs.push(log) + }) + await renderViaHTTP(next.url, 'app-edge') + expect(logs.some((log) => log.includes(`Attempted import error:`))).toBe( + false + ) + }) + it('should handle edge rsc hmr', async () => { const pageFile = 'app/app-edge/page.tsx' const content = await next.readFile(pageFile) @@ -53,9 +64,8 @@ describe('app-dir edge SSR', () => { return html }, /Edge!/) }) - } - - if (!(globalThis as any).isNextDev) { + } else { + // Production tests it('should generate matchers correctly in middleware manifest', async () => { const manifest = JSON.parse( await next.readFile('.next/server/middleware-manifest.json') diff --git a/test/e2e/streaming-ssr/index.test.ts b/test/e2e/streaming-ssr/index.test.ts index 9f500fa2a17c5f0..0b834aac6f56bc0 100644 --- a/test/e2e/streaming-ssr/index.test.ts +++ b/test/e2e/streaming-ssr/index.test.ts @@ -10,11 +10,6 @@ import { renderViaHTTP, } from 'next-test-utils' -const react18Deps = { - react: '^18.0.0', - 'react-dom': '^18.0.0', -} - const isNextProd = !(global as any).isNextDev && !(global as any).isNextDeploy describe('streaming SSR with custom next configs', () => { @@ -31,7 +26,6 @@ describe('streaming SSR with custom next configs', () => { pages: new FileRef(join(__dirname, 'streaming-ssr/pages')), }, nextConfig: require(join(__dirname, 'streaming-ssr/next.config.js')), - dependencies: react18Deps, installCommand: 'npm install', }) }) @@ -116,7 +110,6 @@ if (isNextProd) { 'server.js': new FileRef(join(__dirname, 'custom-server/server.js')), }, nextConfig: require(join(__dirname, 'custom-server/next.config.js')), - dependencies: react18Deps, }) await next.stop() @@ -185,7 +178,6 @@ if (isNextProd) { return config }, }, - dependencies: react18Deps, }) }) afterAll(() => { diff --git a/test/e2e/streaming-ssr/streaming-ssr/pages/router.js b/test/e2e/streaming-ssr/streaming-ssr/pages/router.js index 3407d79f07b2631..762322aa9ecb250 100644 --- a/test/e2e/streaming-ssr/streaming-ssr/pages/router.js +++ b/test/e2e/streaming-ssr/streaming-ssr/pages/router.js @@ -6,6 +6,4 @@ export default () => { return link } -export const config = { - runtime: 'experimental-edge', -} +export const config = { runtime: 'experimental-edge' }