Skip to content

Commit b9c012f

Browse files
styflestyfle
authored
Fix duplicate image src causing canceled request (#33776)
In PR #26968, we added Set of loaded images that was removed in #33474 erroneously. We still need to track loaded images since we can't rely on `img.complete`, especially if the parent uses `react-virtualized`. Tested on https://nextjs.org/showcase
1 parent 2ec2bec commit b9c012f

9 files changed

Lines changed: 169 additions & 5 deletions

File tree

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@
145145
"react-dom": "17.0.2",
146146
"react-dom-18": "npm:react-dom@18.0.0-rc.0",
147147
"react-ssr-prepass": "1.0.8",
148+
"react-virtualized": "9.22.3",
148149
"release": "6.3.0",
149150
"request-promise-core": "1.1.2",
150151
"resolve-from": "5.0.0",

‎packages/next/client/image.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
} from '../server/image-config'
99
import { useIntersection } from './use-intersection'
1010

11+
const loadedImageURLs = new Set<string>()
1112
const allImgs = new Map<
1213
string,
1314
{ src: string; priority: boolean; placeholder: string }
@@ -267,6 +268,7 @@ function handleLoading(
267268
if (!imgRef.current) {
268269
return
269270
}
271+
loadedImageURLs.add(src)
270272
if (placeholder === 'blur') {
271273
img.style.filter = ''
272274
img.style.backgroundSize = ''
@@ -383,7 +385,7 @@ export default function Image({
383385
unoptimized = true
384386
isLazy = false
385387
}
386-
if (typeof window !== 'undefined' && imgRef.current?.complete) {
388+
if (typeof window !== 'undefined' && loadedImageURLs.has(src)) {
387389
isLazy = false
388390
}
389391

‎test/integration/image-component/react-virtualized/pages/index.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import Image from 'next/image'
2+
import img from '../public/test.jpg'
3+
import { WindowScroller, List as VirtualizedList } from 'react-virtualized'
4+
5+
export default function Home() {
6+
return (
7+
<div>
8+
<WindowScroller serverHeight={800}>
9+
{({ height, isScrolling, onChildScroll, scrollTop }) => (
10+
<VirtualizedList
11+
autoHeight
12+
height={height}
13+
isScrolling={isScrolling}
14+
onScroll={onChildScroll}
15+
scrollTop={scrollTop}
16+
width={810}
17+
rowCount={5}
18+
estimatedRowSize={10}
19+
rowHeight={400}
20+
rowRenderer={() => {
21+
return (
22+
<div>
23+
<Image src={img} placeholder="blur" className="thumbnail" />
24+
<Image src={img} className="large" />
25+
</div>
26+
)
27+
}}
28+
overscanRowCount={0}
29+
/>
30+
)}
31+
</WindowScroller>
32+
</div>
33+
)
34+
}

‎test/integration/image-component/react-virtualized/public/test.jpg‎

6.61 KB
Loading

‎test/integration/image-component/react-virtualized/test/index.test.js‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/* eslint-env jest */
2+
3+
import {
4+
findPort,
5+
killApp,
6+
nextBuild,
7+
nextStart,
8+
waitFor,
9+
} from 'next-test-utils'
10+
import webdriver from 'next-webdriver'
11+
import httpProxy from 'http-proxy'
12+
import { join } from 'path'
13+
import { remove } from 'fs-extra'
14+
import http from 'http'
15+
16+
const appDir = join(__dirname, '../')
17+
let appPort
18+
let app
19+
let proxyServer
20+
let cancelCount = 0
21+
describe('react-virtualized wrapping next/image', () => {
22+
describe('production', () => {
23+
beforeAll(async () => {
24+
await remove(join(appDir, '.next'))
25+
await nextBuild(appDir)
26+
const port = await findPort()
27+
app = await nextStart(appDir, port)
28+
appPort = await findPort()
29+
30+
const proxy = httpProxy.createProxyServer({
31+
target: `http://localhost:${port}`,
32+
})
33+
34+
proxyServer = http.createServer(async (req, res) => {
35+
let isComplete = false
36+
37+
if (req.url.startsWith('/_next/image')) {
38+
req.on('close', () => {
39+
if (!isComplete) {
40+
cancelCount++
41+
}
42+
})
43+
console.log('stalling request for', req.url)
44+
await waitFor(3000)
45+
isComplete = true
46+
}
47+
proxy.web(req, res)
48+
})
49+
50+
proxy.on('error', (err) => {
51+
console.warn('Failed to proxy', err)
52+
})
53+
54+
await new Promise((resolve) => {
55+
proxyServer.listen(appPort, () => resolve())
56+
})
57+
})
58+
afterAll(async () => {
59+
proxyServer.close()
60+
await killApp(app)
61+
})
62+
63+
it('should not cancel requests for images', async () => {
64+
// TODO: this test doesnt work unless we can set `disableCache: true`
65+
let browser = await webdriver(appPort, '/', undefined, undefined, true)
66+
expect(cancelCount).toBe(0)
67+
await browser.eval('window.scrollTo({ top: 100, behavior: "smooth" })')
68+
await waitFor(100)
69+
expect(cancelCount).toBe(0)
70+
await browser.eval('window.scrollTo({ top: 200, behavior: "smooth" })')
71+
await waitFor(200)
72+
expect(cancelCount).toBe(0)
73+
await browser.eval('window.scrollTo({ top: 300, behavior: "smooth" })')
74+
await waitFor(300)
75+
expect(cancelCount).toBe(0)
76+
})
77+
})
78+
})

‎test/lib/browsers/base.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export class BrowserInterface {
5959
deleteCookies(): BrowserInterface {
6060
return this
6161
}
62-
async loadPage(url: string): Promise<any> {}
62+
async loadPage(url: string, { disableCache: boolean }): Promise<any> {}
6363
async get(url: string): Promise<void> {}
6464

6565
async getValue(): Promise<any> {}

‎test/lib/browsers/playwright.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class Playwright extends BrowserInterface {
4747
return page.goto(url) as any
4848
}
4949

50-
async loadPage(url: string) {
50+
async loadPage(url: string, opts?: { disableCache: boolean }) {
5151
if (this.activeTrace) {
5252
const traceDir = path.join(__dirname, '../../traces')
5353
const traceOutputPath = path.join(
@@ -85,6 +85,12 @@ class Playwright extends BrowserInterface {
8585
console.error('page error', error)
8686
})
8787

88+
if (opts?.disableCache) {
89+
// TODO: this doesn't seem to work (dev tools does not check the box as expected)
90+
const session = await context.newCDPSession(page)
91+
session.send('Network.setCacheDisabled', { cacheDisabled: true })
92+
}
93+
8894
page.on('websocket', (ws) => {
8995
if (tracePlaywright) {
9096
page

‎test/lib/next-webdriver.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ export default async function webdriver(
4848
appPortOrUrl: string | number,
4949
url: string,
5050
waitHydration = true,
51-
retryWaitHydration = false
51+
retryWaitHydration = false,
52+
disableCache = false
5253
): Promise<BrowserInterface> {
5354
let CurrentInterface: typeof BrowserInterface
5455

@@ -80,7 +81,7 @@ export default async function webdriver(
8081

8182
console.log(`\n> Loading browser with ${fullUrl}\n`)
8283

83-
await browser.loadPage(fullUrl)
84+
await browser.loadPage(fullUrl, { disableCache })
8485
console.log(`\n> Loaded browser with ${fullUrl}\n`)
8586

8687
// Wait for application to hydrate

‎yarn.lock‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2281,6 +2281,13 @@
22812281
dependencies:
22822282
regenerator-runtime "^0.13.4"
22832283

2284+
"@babel/runtime@^7.7.2", "@babel/runtime@^7.8.7":
2285+
version "7.16.7"
2286+
resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.16.7.tgz#03ff99f64106588c9c403c6ecb8c3bafbbdff1fa"
2287+
integrity sha512-9E9FJowqAsytyOY6LG+1KuueckRL+aQW+mKvXRXnuFGyRAyepJPmEo9vgMfXUA6O9u3IeEdv9MAkppFcaQwogQ==
2288+
dependencies:
2289+
regenerator-runtime "^0.13.4"
2290+
22842291
"@babel/template@^7.10.4", "@babel/template@^7.12.7", "@babel/template@^7.3.3":
22852292
version "7.12.7"
22862293
resolved "https://registry.yarnpkg.com/@babel/template/-/template-7.12.7.tgz#c817233696018e39fbb6c491d2fb684e05ed43bc"
@@ -7255,6 +7262,11 @@ clor@^5.1.0:
72557262
version "5.2.0"
72567263
resolved "https://registry.yarnpkg.com/clor/-/clor-5.2.0.tgz#9ddc74e7e86728cfcd05a80546ba58d317b81035"
72577264

7265+
clsx@^1.0.4:
7266+
version "1.1.1"
7267+
resolved "https://registry.yarnpkg.com/clsx/-/clsx-1.1.1.tgz#98b3134f9abbdf23b2663491ace13c5c03a73188"
7268+
integrity sha512-6/bPho624p3S2pMyvP5kKBPXnI3ufHLObBFCfgx+LkeR5lg2XYy2hqZqUf45ypD8COn2bhgGJSUE+l5dhNBieA==
7269+
72587270
cmd-shim@^4.1.0:
72597271
version "4.1.0"
72607272
resolved "https://registry.yarnpkg.com/cmd-shim/-/cmd-shim-4.1.0.tgz#b3a904a6743e9fede4148c6f3800bf2a08135bdd"
@@ -8161,6 +8173,11 @@ csstype@^2.2.0:
81618173
version "2.6.8"
81628174
resolved "https://registry.yarnpkg.com/csstype/-/csstype-2.6.8.tgz#0fb6fc2417ffd2816a418c9336da74d7f07db431"
81638175

8176+
csstype@^3.0.2:
8177+
version "3.0.10"
8178+
resolved "https://registry.yarnpkg.com/csstype/-/csstype-3.0.10.tgz#2ad3a7bed70f35b965707c092e5f30b327c290e5"
8179+
integrity sha512-2u44ZG2OcNUO9HDp/Jl8C07x6pU/eTR3ncV91SiK3dhG9TWvRVsCoJw14Ckx5DgWkzGA3waZWO3d7pgqpUI/XA==
8180+
81648181
currently-unhandled@^0.4.1:
81658182
version "0.4.1"
81668183
resolved "https://registry.yarnpkg.com/currently-unhandled/-/currently-unhandled-0.4.1.tgz#988df33feab191ef799a61369dd76c17adf957ea"
@@ -8513,6 +8530,14 @@ dom-accessibility-api@^0.5.4:
85138530
resolved "https://registry.yarnpkg.com/dom-accessibility-api/-/dom-accessibility-api-0.5.4.tgz#b06d059cdd4a4ad9a79275f9d414a5c126241166"
85148531
integrity sha512-TvrjBckDy2c6v6RLxPv5QXOnU+SmF9nBII5621Ve5fu6Z/BDrENurBEvlC1f44lKEUVqOpK4w9E5Idc5/EgkLQ==
85158532

8533+
dom-helpers@^5.1.3:
8534+
version "5.2.1"
8535+
resolved "https://registry.yarnpkg.com/dom-helpers/-/dom-helpers-5.2.1.tgz#d9400536b2bf8225ad98fe052e029451ac40e902"
8536+
integrity sha512-nRCa7CK3VTrM2NmGkIy4cbK7IZlgBE/PYMn55rrXefr5xXDP0LdtfPnblFDoVdcAfslJ7or6iqAUnx0CCGIWQA==
8537+
dependencies:
8538+
"@babel/runtime" "^7.8.7"
8539+
csstype "^3.0.2"
8540+
85168541
dom-serializer@0:
85178542
version "0.2.2"
85188543
resolved "https://registry.yarnpkg.com/dom-serializer/-/dom-serializer-0.2.2.tgz#1afb81f533717175d478655debc5e332d9f9bb51"
@@ -16967,6 +16992,11 @@ react-is@^17.0.1:
1696716992
resolved "https://registry.yarnpkg.com/react-is/-/react-is-17.0.1.tgz#5b3531bd76a645a4c9fb6e693ed36419e3301339"
1696816993
integrity sha512-NAnt2iGDXohE5LI7uBnLnqvLQMtzhkiAOLXTmv+qnF9Ky7xAPcX8Up/xWIhxvLVGJvuLiNc4xQLtuqDRzb4fSA==
1696916994

16995+
react-lifecycles-compat@^3.0.4:
16996+
version "3.0.4"
16997+
resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362"
16998+
integrity sha512-fBASbA6LnOU9dOU2eW7aQ8xmYBSXUIWr+UmF9b1efZBazGNO+rcXT/icdKnYm2pTwcRylVUYwW7H1PHfLekVzA==
16999+
1697017000
react-refresh@0.8.3:
1697117001
version "0.8.3"
1697217002
resolved "https://registry.yarnpkg.com/react-refresh/-/react-refresh-0.8.3.tgz#721d4657672d400c5e3c75d063c4a85fb2d5d68f"
@@ -16988,6 +17018,18 @@ react-ssr-prepass@1.0.8:
1698817018
dependencies:
1698917019
object-is "^1.0.1"
1699017020

17021+
react-virtualized@9.22.3:
17022+
version "9.22.3"
17023+
resolved "https://registry.yarnpkg.com/react-virtualized/-/react-virtualized-9.22.3.tgz#f430f16beb0a42db420dbd4d340403c0de334421"
17024+
integrity sha512-MKovKMxWTcwPSxE1kK1HcheQTWfuCxAuBoSTf2gwyMM21NdX/PXUhnoP8Uc5dRKd+nKm8v41R36OellhdCpkrw==
17025+
dependencies:
17026+
"@babel/runtime" "^7.7.2"
17027+
clsx "^1.0.4"
17028+
dom-helpers "^5.1.3"
17029+
loose-envify "^1.4.0"
17030+
prop-types "^15.7.2"
17031+
react-lifecycles-compat "^3.0.4"
17032+
1699117033
react@17.0.2:
1699217034
version "17.0.2"
1699317035
resolved "https://registry.yarnpkg.com/react/-/react-17.0.2.tgz#d0b5cc516d29eb3eee383f75b62864cfb6800037"

0 commit comments

Comments
 (0)