Skip to content

Commit

Permalink
Fix duplicate image src causing canceled request (vercel#33776)
Browse files Browse the repository at this point in the history
In PR vercel#26968, we added Set of loaded images that was removed in vercel#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
  • Loading branch information
styfle authored and natew committed Feb 16, 2022
1 parent b0bd456 commit e3aa980
Show file tree
Hide file tree
Showing 9 changed files with 169 additions and 5 deletions.
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -145,6 +145,7 @@
"react-dom": "17.0.2",
"react-dom-18": "npm:react-dom@18.0.0-rc.0",
"react-ssr-prepass": "1.0.8",
"react-virtualized": "9.22.3",
"release": "6.3.0",
"request-promise-core": "1.1.2",
"resolve-from": "5.0.0",
Expand Down
4 changes: 3 additions & 1 deletion packages/next/client/image.tsx
Expand Up @@ -8,6 +8,7 @@ import {
} from '../server/image-config'
import { useIntersection } from './use-intersection'

const loadedImageURLs = new Set<string>()
const allImgs = new Map<
string,
{ src: string; priority: boolean; placeholder: string }
Expand Down Expand Up @@ -267,6 +268,7 @@ function handleLoading(
if (!imgRef.current) {
return
}
loadedImageURLs.add(src)
if (placeholder === 'blur') {
img.style.filter = ''
img.style.backgroundSize = ''
Expand Down Expand Up @@ -383,7 +385,7 @@ export default function Image({
unoptimized = true
isLazy = false
}
if (typeof window !== 'undefined' && imgRef.current?.complete) {
if (typeof window !== 'undefined' && loadedImageURLs.has(src)) {
isLazy = false
}

Expand Down
34 changes: 34 additions & 0 deletions test/integration/image-component/react-virtualized/pages/index.js
@@ -0,0 +1,34 @@
import Image from 'next/image'
import img from '../public/test.jpg'
import { WindowScroller, List as VirtualizedList } from 'react-virtualized'

export default function Home() {
return (
<div>
<WindowScroller serverHeight={800}>
{({ height, isScrolling, onChildScroll, scrollTop }) => (
<VirtualizedList
autoHeight
height={height}
isScrolling={isScrolling}
onScroll={onChildScroll}
scrollTop={scrollTop}
width={810}
rowCount={5}
estimatedRowSize={10}
rowHeight={400}
rowRenderer={() => {
return (
<div>
<Image src={img} placeholder="blur" className="thumbnail" />
<Image src={img} className="large" />
</div>
)
}}
overscanRowCount={0}
/>
)}
</WindowScroller>
</div>
)
}
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
@@ -0,0 +1,78 @@
/* eslint-env jest */

import {
findPort,
killApp,
nextBuild,
nextStart,
waitFor,
} from 'next-test-utils'
import webdriver from 'next-webdriver'
import httpProxy from 'http-proxy'
import { join } from 'path'
import { remove } from 'fs-extra'
import http from 'http'

const appDir = join(__dirname, '../')
let appPort
let app
let proxyServer
let cancelCount = 0
describe('react-virtualized wrapping next/image', () => {
describe('production', () => {
beforeAll(async () => {
await remove(join(appDir, '.next'))
await nextBuild(appDir)
const port = await findPort()
app = await nextStart(appDir, port)
appPort = await findPort()

const proxy = httpProxy.createProxyServer({
target: `http://localhost:${port}`,
})

proxyServer = http.createServer(async (req, res) => {
let isComplete = false

if (req.url.startsWith('/_next/image')) {
req.on('close', () => {
if (!isComplete) {
cancelCount++
}
})
console.log('stalling request for', req.url)
await waitFor(3000)
isComplete = true
}
proxy.web(req, res)
})

proxy.on('error', (err) => {
console.warn('Failed to proxy', err)
})

await new Promise((resolve) => {
proxyServer.listen(appPort, () => resolve())
})
})
afterAll(async () => {
proxyServer.close()
await killApp(app)
})

it('should not cancel requests for images', async () => {
// TODO: this test doesnt work unless we can set `disableCache: true`
let browser = await webdriver(appPort, '/', undefined, undefined, true)
expect(cancelCount).toBe(0)
await browser.eval('window.scrollTo({ top: 100, behavior: "smooth" })')
await waitFor(100)
expect(cancelCount).toBe(0)
await browser.eval('window.scrollTo({ top: 200, behavior: "smooth" })')
await waitFor(200)
expect(cancelCount).toBe(0)
await browser.eval('window.scrollTo({ top: 300, behavior: "smooth" })')
await waitFor(300)
expect(cancelCount).toBe(0)
})
})
})
2 changes: 1 addition & 1 deletion test/lib/browsers/base.ts
Expand Up @@ -59,7 +59,7 @@ export class BrowserInterface {
deleteCookies(): BrowserInterface {
return this
}
async loadPage(url: string): Promise<any> {}
async loadPage(url: string, { disableCache: boolean }): Promise<any> {}
async get(url: string): Promise<void> {}

async getValue(): Promise<any> {}
Expand Down
8 changes: 7 additions & 1 deletion test/lib/browsers/playwright.ts
Expand Up @@ -47,7 +47,7 @@ class Playwright extends BrowserInterface {
return page.goto(url) as any
}

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

if (opts?.disableCache) {
// TODO: this doesn't seem to work (dev tools does not check the box as expected)
const session = await context.newCDPSession(page)
session.send('Network.setCacheDisabled', { cacheDisabled: true })
}

page.on('websocket', (ws) => {
if (tracePlaywright) {
page
Expand Down
5 changes: 3 additions & 2 deletions test/lib/next-webdriver.ts
Expand Up @@ -48,7 +48,8 @@ export default async function webdriver(
appPortOrUrl: string | number,
url: string,
waitHydration = true,
retryWaitHydration = false
retryWaitHydration = false,
disableCache = false
): Promise<BrowserInterface> {
let CurrentInterface: typeof BrowserInterface

Expand Down Expand Up @@ -80,7 +81,7 @@ export default async function webdriver(

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

await browser.loadPage(fullUrl)
await browser.loadPage(fullUrl, { disableCache })
console.log(`\n> Loaded browser with ${fullUrl}\n`)

// Wait for application to hydrate
Expand Down
42 changes: 42 additions & 0 deletions yarn.lock
Expand Up @@ -2281,6 +2281,13 @@
dependencies:
regenerator-runtime "^0.13.4"

"@babel/runtime@^7.7.2", "@babel/runtime@^7.8.7":
version "7.16.7"
resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.16.7.tgz#03ff99f64106588c9c403c6ecb8c3bafbbdff1fa"
integrity sha512-9E9FJowqAsytyOY6LG+1KuueckRL+aQW+mKvXRXnuFGyRAyepJPmEo9vgMfXUA6O9u3IeEdv9MAkppFcaQwogQ==
dependencies:
regenerator-runtime "^0.13.4"

"@babel/template@^7.10.4", "@babel/template@^7.12.7", "@babel/template@^7.3.3":
version "7.12.7"
resolved "https://registry.yarnpkg.com/@babel/template/-/template-7.12.7.tgz#c817233696018e39fbb6c491d2fb684e05ed43bc"
Expand Down Expand Up @@ -7255,6 +7262,11 @@ clor@^5.1.0:
version "5.2.0"
resolved "https://registry.yarnpkg.com/clor/-/clor-5.2.0.tgz#9ddc74e7e86728cfcd05a80546ba58d317b81035"

clsx@^1.0.4:
version "1.1.1"
resolved "https://registry.yarnpkg.com/clsx/-/clsx-1.1.1.tgz#98b3134f9abbdf23b2663491ace13c5c03a73188"
integrity sha512-6/bPho624p3S2pMyvP5kKBPXnI3ufHLObBFCfgx+LkeR5lg2XYy2hqZqUf45ypD8COn2bhgGJSUE+l5dhNBieA==

cmd-shim@^4.1.0:
version "4.1.0"
resolved "https://registry.yarnpkg.com/cmd-shim/-/cmd-shim-4.1.0.tgz#b3a904a6743e9fede4148c6f3800bf2a08135bdd"
Expand Down Expand Up @@ -8161,6 +8173,11 @@ csstype@^2.2.0:
version "2.6.8"
resolved "https://registry.yarnpkg.com/csstype/-/csstype-2.6.8.tgz#0fb6fc2417ffd2816a418c9336da74d7f07db431"

csstype@^3.0.2:
version "3.0.10"
resolved "https://registry.yarnpkg.com/csstype/-/csstype-3.0.10.tgz#2ad3a7bed70f35b965707c092e5f30b327c290e5"
integrity sha512-2u44ZG2OcNUO9HDp/Jl8C07x6pU/eTR3ncV91SiK3dhG9TWvRVsCoJw14Ckx5DgWkzGA3waZWO3d7pgqpUI/XA==

currently-unhandled@^0.4.1:
version "0.4.1"
resolved "https://registry.yarnpkg.com/currently-unhandled/-/currently-unhandled-0.4.1.tgz#988df33feab191ef799a61369dd76c17adf957ea"
Expand Down Expand Up @@ -8513,6 +8530,14 @@ dom-accessibility-api@^0.5.4:
resolved "https://registry.yarnpkg.com/dom-accessibility-api/-/dom-accessibility-api-0.5.4.tgz#b06d059cdd4a4ad9a79275f9d414a5c126241166"
integrity sha512-TvrjBckDy2c6v6RLxPv5QXOnU+SmF9nBII5621Ve5fu6Z/BDrENurBEvlC1f44lKEUVqOpK4w9E5Idc5/EgkLQ==

dom-helpers@^5.1.3:
version "5.2.1"
resolved "https://registry.yarnpkg.com/dom-helpers/-/dom-helpers-5.2.1.tgz#d9400536b2bf8225ad98fe052e029451ac40e902"
integrity sha512-nRCa7CK3VTrM2NmGkIy4cbK7IZlgBE/PYMn55rrXefr5xXDP0LdtfPnblFDoVdcAfslJ7or6iqAUnx0CCGIWQA==
dependencies:
"@babel/runtime" "^7.8.7"
csstype "^3.0.2"

dom-serializer@0:
version "0.2.2"
resolved "https://registry.yarnpkg.com/dom-serializer/-/dom-serializer-0.2.2.tgz#1afb81f533717175d478655debc5e332d9f9bb51"
Expand Down Expand Up @@ -16967,6 +16992,11 @@ react-is@^17.0.1:
resolved "https://registry.yarnpkg.com/react-is/-/react-is-17.0.1.tgz#5b3531bd76a645a4c9fb6e693ed36419e3301339"
integrity sha512-NAnt2iGDXohE5LI7uBnLnqvLQMtzhkiAOLXTmv+qnF9Ky7xAPcX8Up/xWIhxvLVGJvuLiNc4xQLtuqDRzb4fSA==

react-lifecycles-compat@^3.0.4:
version "3.0.4"
resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362"
integrity sha512-fBASbA6LnOU9dOU2eW7aQ8xmYBSXUIWr+UmF9b1efZBazGNO+rcXT/icdKnYm2pTwcRylVUYwW7H1PHfLekVzA==

react-refresh@0.8.3:
version "0.8.3"
resolved "https://registry.yarnpkg.com/react-refresh/-/react-refresh-0.8.3.tgz#721d4657672d400c5e3c75d063c4a85fb2d5d68f"
Expand All @@ -16988,6 +17018,18 @@ react-ssr-prepass@1.0.8:
dependencies:
object-is "^1.0.1"

react-virtualized@9.22.3:
version "9.22.3"
resolved "https://registry.yarnpkg.com/react-virtualized/-/react-virtualized-9.22.3.tgz#f430f16beb0a42db420dbd4d340403c0de334421"
integrity sha512-MKovKMxWTcwPSxE1kK1HcheQTWfuCxAuBoSTf2gwyMM21NdX/PXUhnoP8Uc5dRKd+nKm8v41R36OellhdCpkrw==
dependencies:
"@babel/runtime" "^7.7.2"
clsx "^1.0.4"
dom-helpers "^5.1.3"
loose-envify "^1.4.0"
prop-types "^15.7.2"
react-lifecycles-compat "^3.0.4"

react@17.0.2:
version "17.0.2"
resolved "https://registry.yarnpkg.com/react/-/react-17.0.2.tgz#d0b5cc516d29eb3eee383f75b62864cfb6800037"
Expand Down

0 comments on commit e3aa980

Please sign in to comment.