Skip to content

Commit

Permalink
Fix module error for findDOMNode on edge (#43998)
Browse files Browse the repository at this point in the history
## 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)
  • Loading branch information
huozhi committed Dec 13, 2022
1 parent 04c2509 commit 60dd6df
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 20 deletions.
14 changes: 8 additions & 6 deletions packages/next/client/components/layout-router.tsx
Expand Up @@ -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,
Expand Down Expand Up @@ -80,8 +80,10 @@ function walkAddRefetch(
* Wraps ReactDOM.findDOMNode with additional logic to hide React Strict Mode warning
*/
function findDOMNode(
instance: Parameters<typeof ReactDOMfindDOMNode>[0]
): ReturnType<typeof ReactDOMfindDOMNode> {
instance: Parameters<typeof ReactDOM.findDOMNode>[0]
): ReturnType<typeof ReactDOM.findDOMNode> {
// 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
Expand All @@ -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)
}

/**
Expand Down
16 changes: 13 additions & 3 deletions test/e2e/app-dir/app-edge.test.ts
Expand Up @@ -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)
Expand All @@ -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')
Expand Down
8 changes: 0 additions & 8 deletions test/e2e/streaming-ssr/index.test.ts
Expand Up @@ -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', () => {
Expand All @@ -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',
})
})
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -185,7 +178,6 @@ if (isNextProd) {
return config
},
},
dependencies: react18Deps,
})
})
afterAll(() => {
Expand Down
4 changes: 1 addition & 3 deletions test/e2e/streaming-ssr/streaming-ssr/pages/router.js
Expand Up @@ -6,6 +6,4 @@ export default () => {
return <Link href="/">link</Link>
}

export const config = {
runtime: 'experimental-edge',
}
export const config = { runtime: 'experimental-edge' }

0 comments on commit 60dd6df

Please sign in to comment.