Skip to content

Commit 70d5901

Browse files
committed
Run link-ref tests in /app and /pages
We need to handle ref cleanups in a way that is compatible with React 18 and 19. Forking these into -pages (18) and -app (19). Combining them into a single app lead to a different behavior with regards to preload.
1 parent 214984e commit 70d5901

File tree

17 files changed

+327
-0
lines changed

17 files changed

+327
-0
lines changed
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
'use client'
2+
import React from 'react'
3+
import Link from 'next/link'
4+
import { useCallback, useRef, useEffect, useState } from 'react'
5+
import { flushSync } from 'react-dom'
6+
7+
export default function Page() {
8+
const [isVisible, setIsVisible] = useState(true)
9+
10+
const statusRef = useRef({ wasInitialized: false, wasCleanedUp: false })
11+
12+
const refWithCleanup = useCallback((el) => {
13+
if (!el) {
14+
console.error(
15+
'callback refs that returned a cleanup should never be called with null'
16+
)
17+
return
18+
}
19+
20+
statusRef.current.wasInitialized = true
21+
return () => {
22+
statusRef.current.wasCleanedUp = true
23+
}
24+
}, [])
25+
26+
useEffect(() => {
27+
const timeout = setTimeout(
28+
() => {
29+
flushSync(() => {
30+
setIsVisible(false)
31+
})
32+
if (!statusRef.current.wasInitialized) {
33+
console.error('callback ref was not initialized')
34+
}
35+
if (!statusRef.current.wasCleanedUp) {
36+
console.error('callback ref was not cleaned up')
37+
}
38+
},
39+
100 // if we hide the Link too quickly, the prefetch won't fire, failing a test
40+
)
41+
return () => clearTimeout(timeout)
42+
}, [])
43+
44+
if (!isVisible) {
45+
return null
46+
}
47+
48+
return (
49+
<Link href="/" ref={refWithCleanup}>
50+
Click me
51+
</Link>
52+
)
53+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
'use client'
2+
import React from 'react'
3+
import Link from 'next/link'
4+
5+
export default () => {
6+
const myRef = React.createRef(null)
7+
8+
React.useEffect(() => {
9+
if (!myRef.current) {
10+
console.error(`ref wasn't updated`)
11+
}
12+
})
13+
14+
return (
15+
<Link
16+
href="/"
17+
ref={(el) => {
18+
myRef.current = el
19+
}}
20+
>
21+
Click me
22+
</Link>
23+
)
24+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
'use client'
2+
import React from 'react'
3+
import Link from 'next/link'
4+
5+
export default () => {
6+
const myRef = React.createRef(null)
7+
8+
React.useEffect(() => {
9+
if (!myRef.current) {
10+
console.error(`ref wasn't updated`)
11+
}
12+
})
13+
14+
return (
15+
<Link href="/" ref={myRef}>
16+
Click me
17+
</Link>
18+
)
19+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
'use client'
2+
import React from 'react'
3+
import Link from 'next/link'
4+
5+
class MyLink extends React.Component {
6+
render() {
7+
return <a {...this.props}>Click me</a>
8+
}
9+
}
10+
11+
export default () => (
12+
<Link href="/" passHref legacyBehavior>
13+
<MyLink />
14+
</Link>
15+
)
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
'use client'
2+
import React, { useCallback, useEffect, useRef, useState } from 'react'
3+
import Link from 'next/link'
4+
5+
const useClickAway = (ref, onClickAway) => {
6+
useEffect(() => {
7+
const handler = (event) => {
8+
const el = ref.current
9+
10+
// when menu is open and clicked inside menu, A is expected to be false
11+
// when menu is open and clicked outside menu, A is expected to be true
12+
console.log('A', el && !el.contains(event.target))
13+
14+
el && !el.contains(event.target) && onClickAway(event)
15+
}
16+
17+
let timeoutID = setTimeout(() => {
18+
timeoutID = null
19+
document.addEventListener('click', handler)
20+
}, 0)
21+
22+
return () => {
23+
if (timeoutID != null) {
24+
clearTimeout(timeoutID)
25+
}
26+
document.removeEventListener('click', handler)
27+
}
28+
}, [onClickAway, ref])
29+
}
30+
31+
export default function App() {
32+
const [open, setOpen] = useState(false)
33+
34+
const menuRef = useRef(null)
35+
36+
const onClickAway = useCallback(() => {
37+
console.log('click away, open', open)
38+
if (open) {
39+
setOpen(false)
40+
}
41+
}, [open])
42+
43+
useClickAway(menuRef, onClickAway)
44+
45+
return (
46+
<div>
47+
<div id="click-me" onClick={() => setOpen(true)}>
48+
Open Menu
49+
</div>
50+
{open && (
51+
<div ref={menuRef} id="the-menu">
52+
<Link href="/">some link</Link>
53+
</div>
54+
)}
55+
</div>
56+
)
57+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
'use client'
2+
import React from 'react'
3+
import Link from 'next/link'
4+
5+
const MyLink = React.forwardRef((props, ref) => (
6+
<a {...props} ref={ref}>
7+
Click me
8+
</a>
9+
))
10+
11+
export default () => (
12+
<Link href="/" passHref legacyBehavior>
13+
<MyLink />
14+
</Link>
15+
)
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export default function RootLayout({ children }) {
2+
return (
3+
<html>
4+
<body>{children}</body>
5+
</html>
6+
)
7+
}
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
/* eslint-env jest */
2+
3+
import { join } from 'path'
4+
import webdriver from 'next-webdriver'
5+
import {
6+
retry,
7+
findPort,
8+
launchApp,
9+
killApp,
10+
nextStart,
11+
nextBuild,
12+
waitFor,
13+
} from 'next-test-utils'
14+
15+
let app
16+
let appPort
17+
const appDir = join(__dirname, '..')
18+
19+
const noError = async (pathname) => {
20+
const browser = await webdriver(appPort, '/')
21+
await browser.eval(`(function() {
22+
window.caughtErrors = []
23+
const origError = window.console.error
24+
window.console.error = function (format) {
25+
window.caughtErrors.push(format)
26+
origError(arguments)
27+
}
28+
window.next.router.replace('${pathname}')
29+
})()`)
30+
await waitFor(1000)
31+
const errors = await browser.eval(`window.caughtErrors`)
32+
expect(errors).toEqual([])
33+
await browser.close()
34+
}
35+
36+
const didPrefetch = async (pathname) => {
37+
const browser = await webdriver(appPort, pathname)
38+
39+
await retry(async () => {
40+
const links = await browser.elementsByCss('link[rel=prefetch]')
41+
42+
const hrefs = await Promise.all(
43+
links.map((link) => link.getAttribute('href'))
44+
)
45+
46+
// expect one of the href contain string "index"
47+
expect(hrefs).toEqual(
48+
expect.arrayContaining([expect.stringContaining('index')])
49+
)
50+
})
51+
52+
await browser.close()
53+
}
54+
55+
function runCommonTests() {
56+
// See https://github.com/vercel/next.js/issues/18437
57+
it('should not have a race condition with a click handler', async () => {
58+
const browser = await webdriver(appPort, '/click-away-race-condition')
59+
await browser.elementByCss('#click-me').click()
60+
await browser.waitForElementByCss('#the-menu')
61+
})
62+
}
63+
64+
describe('Invalid hrefs', () => {
65+
;(process.env.TURBOPACK_BUILD ? describe.skip : describe)(
66+
'development mode',
67+
() => {
68+
beforeAll(async () => {
69+
appPort = await findPort()
70+
app = await launchApp(appDir, appPort)
71+
})
72+
afterAll(() => killApp(app))
73+
74+
runCommonTests()
75+
76+
it('should not show error for function component with forwardRef', async () => {
77+
await noError('/function')
78+
})
79+
80+
it('should not show error for class component as child of next/link', async () => {
81+
await noError('/class')
82+
})
83+
84+
it('should handle child ref with React.createRef', async () => {
85+
await noError('/child-ref')
86+
})
87+
88+
it('should handle child ref that is a function', async () => {
89+
await noError('/child-ref-func')
90+
})
91+
92+
it('should handle child ref that is a function that returns a cleanup function', async () => {
93+
await noError('/child-ref-func-cleanup')
94+
})
95+
}
96+
)
97+
;(process.env.TURBOPACK_DEV ? describe.skip : describe)(
98+
'production mode',
99+
() => {
100+
beforeAll(async () => {
101+
await nextBuild(appDir)
102+
appPort = await findPort()
103+
app = await nextStart(appDir, appPort)
104+
})
105+
afterAll(() => killApp(app))
106+
107+
runCommonTests()
108+
109+
// TODO: Why no preload in /app?
110+
it.failing('should preload with forwardRef', async () => {
111+
await didPrefetch('/function')
112+
})
113+
114+
// TODO: Why no preload in /app?
115+
it.failing(
116+
'should preload with child ref with React.createRef',
117+
async () => {
118+
await didPrefetch('/child-ref')
119+
}
120+
)
121+
122+
// TODO: Why no preload in /app?
123+
it.failing('should preload with child ref with function', async () => {
124+
await didPrefetch('/child-ref-func')
125+
})
126+
127+
// TODO: Why no preload in /app?
128+
it.failing(
129+
'should preload with child ref with function that returns a cleanup function',
130+
async () => {
131+
await didPrefetch('/child-ref-func-cleanup')
132+
}
133+
)
134+
}
135+
)
136+
})
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export default () => 'hi'

0 commit comments

Comments
 (0)