From b9c012f49d36f998d06922ad557241b16e3a4024 Mon Sep 17 00:00:00 2001 From: Steven Date: Fri, 28 Jan 2022 18:50:36 -0500 Subject: [PATCH] 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 --- package.json | 1 + packages/next/client/image.tsx | 4 +- .../react-virtualized/pages/index.js | 34 ++++++++ .../react-virtualized/public/test.jpg | Bin 0 -> 6765 bytes .../react-virtualized/test/index.test.js | 78 ++++++++++++++++++ test/lib/browsers/base.ts | 2 +- test/lib/browsers/playwright.ts | 8 +- test/lib/next-webdriver.ts | 5 +- yarn.lock | 42 ++++++++++ 9 files changed, 169 insertions(+), 5 deletions(-) create mode 100644 test/integration/image-component/react-virtualized/pages/index.js create mode 100644 test/integration/image-component/react-virtualized/public/test.jpg create mode 100644 test/integration/image-component/react-virtualized/test/index.test.js diff --git a/package.json b/package.json index e38083a606feead..0b43fbd204ede71 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/packages/next/client/image.tsx b/packages/next/client/image.tsx index 0980f8abd535b7c..0a5513bfcb5434a 100644 --- a/packages/next/client/image.tsx +++ b/packages/next/client/image.tsx @@ -8,6 +8,7 @@ import { } from '../server/image-config' import { useIntersection } from './use-intersection' +const loadedImageURLs = new Set() const allImgs = new Map< string, { src: string; priority: boolean; placeholder: string } @@ -267,6 +268,7 @@ function handleLoading( if (!imgRef.current) { return } + loadedImageURLs.add(src) if (placeholder === 'blur') { img.style.filter = '' img.style.backgroundSize = '' @@ -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 } diff --git a/test/integration/image-component/react-virtualized/pages/index.js b/test/integration/image-component/react-virtualized/pages/index.js new file mode 100644 index 000000000000000..5f14f13e2f33f54 --- /dev/null +++ b/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 ( +
+ + {({ height, isScrolling, onChildScroll, scrollTop }) => ( + { + return ( +
+ + +
+ ) + }} + overscanRowCount={0} + /> + )} + +
+ ) +} diff --git a/test/integration/image-component/react-virtualized/public/test.jpg b/test/integration/image-component/react-virtualized/public/test.jpg new file mode 100644 index 0000000000000000000000000000000000000000..d536c882412ed3df0dc162823ca5146bcc033499 GIT binary patch literal 6765 zcmeHK2~-nT7rx0%2m%J#v>-x6R0=4dR1uMV3krk?xD*$JK%#*_5>`=Mq0&}Bt+uvD zP_X#B;ZodCL7{GyilVZ(22r4jfQlku=6@4bJyo#&zvukt{4ZzTd~@G@_ulWmJMT@1 z3gSKt@o;6i0)+zLj($K$VTMaAKLo(j6N~{s5vUY(z!0LKA0+bumt%l2=ng>5q;^Xv zX_;6rCI^WIuwwIs5}}wUj9^Y2Zw^+DEKi)YfSMeSmct>}M|@YA3WxCe6@z|!((1UJ zsHWGkoSYW0Io__U87}ew=@o$y5dta`AS_%W;-chvT=_s}*UxYt%@4saK@{RFZ~CZL5iglJ9o>x(_cg(R&Lkd> z@ZO+6mzf9~B3u>C_xI|;vIvPI2Vqn#RD-A`ehvtux}v&=h+O>;Ms}zoUX*(`-Wt#I zorUB>k^F4xVnl#}sP#PgiUI7#{C#ep7dgmn)Y;L$8nNJc&gFht@xFCc@s1Jg0cmqt}fEzfXdjyEkNC@yj zfFxWr%0&_`dg|60C!Z&VB}mSPX!)2J^=!Fj=ge+hCWInsIMm5?gTP5|CqyAjJa~en zydIlOa6(T}NEZ4YJDsuAci9o*!*FwaBD$vHGw^A+6+Q)+xE*ef+v3hhIt8EFW1EfU zbTcC3sYhNq?L;DvT)Cb<;(i8klt3WrrAR{v;vNfcWhG4~%BXi_m1qG!=t^o+pIq_L z%q2Y<u823gk1xP!{-zGq(@taeZx^PdNESueTfcv4Ap_^9dp0X*#`9G7H>fua{o1%CuK% zUT)rCe#3mbdA9juY$KM3ox+Y|C$P)F#0s%9SOInp%f<40^gmQXJ!=nSU;=kI-y z?+U-i5?TYwU{nG8UXO3pfFFvO4>8E52<4lsw{VCZh^DjsctK>=DTex zxF|R)H~>?@SYe8Sg@Ol(yWeGnv1n`x>RtNAhU%k7<1MCK2{)EJPrykS5hvn@@+8a& z=H`=`4(RCPGFjn4<4u`?0s&J#BxZ`ZVy-Bf8$2G!bCaA0@SGz*4=F>h^vWcj0MnkL zy|1)aHa7}juNYvMWv|Q#?Uh;?0LLZ;MTw$2?V*FZ1V9`zaf1ArqT-15ue${C9PMND z4FGS_38H-mLA=RA_HP3e3W!2bQ>3I((lCkvP}L}y8ignZbktC26nX113=}Gc(-i26 zgOrq!Lf#PcVS-^)9HY_54+_dMG!D2LO?{+=gMx-nAl)ERbHd8>?TVuu51!HDTx~L( zxJa~WkkZg$Uuf%$9y8YHOJmEgCQY`QV(sMY;_Bwk@|fxC=RXV43kv3jg!1@{#geG# znAo`Z)oa!!C4aLnWy8izKWyIe<4;?6WM%LCHD}kqb{{(YTi%hQ$Bv&kTU7k}x$_q; zUbO`#DHujmL~FM+12dijYT$2Pa}{K%HY9+ zwKTL0H8c$Ut+(i(pN7A@59n^qn*XrrO3}Hh%Dh`U-_$%1FkgB#v}Tkp zEv0AktJ@K6Wdx zR#mAJJ#IVcZ<1j)^`X$2o>BZlucgJ*!cC~|pw!^Z-kp_+t*}$7wxLO-JL;9i)&ykF zThWkyDYq%N#g+h%Mjq@)F`{bkSRcD>(54CQJ7?>y(NoOboZz`*dgkJ16;`(MW1P>o zrfpM|mbRy~9Xh73ADh^|#C`Il$FlUr%DH>S+j3=DcURbokQvg{KrW2SF*iIX-y@eC-T5|DK##QDKmLB}7hQF`Z&#kXbsuA>_-OC6Vv zUDmfTd>Q%6@@0b)@=Q5SCQe4ibsh(%Iq0g{s|=XnK)1j>RpA+tyYCg{%>C}-M4H~+ z>~_k2v2S3GNWAaVqT7-iv$tWU*U=lly?)W2zUe2A zjG&6th`3DEbl#q-%^8t9@0H}-QDG@-*~-=|RlBg;av*Ogg&5!_{K_n8x!$CkCHwD~ zHI5w~NIw)KES9ald-7Jr5E^V0fNABGH^Ff_GH7BHFQ0=Yy`NJ11D$x`dH_h0-!Ns`n1+&Q| z;c#`B`aEpb<}+uWJwGwVu6RvTA|v@WTY;wOW8dGUPYmP)*#Dq#xt{FSO}t$B=OqE| zvhVJlEm&96t^_SYN^C6egyZc$3+jh0oEj|J(_!+)yWm=WRc~!}*Qosdo|i2DFC*-M zhsm+#+T?eW{MLuD^E=BlL_59V zzkX=D-~P&d>sgxyC~U+E7E<5yW3oW78&*t$hZz@d5jj|(#0A{;N#`S6$ks{XFp zeGoUVpY_rw`Z?YH>CvEKXso$(TXu#hZlBJ3)~$Q5Ih^Ndeb2A#QQ3Z14%gT_Kra)$ zXJ{5yGj6)~v1^Tw%AO_}u1(2Ebe#50ji1gdvvsHS+C2|FzPV@13VaizzOsNC_p)tP zQYpnnll{Jn-rt${czg4`lzr9iBlD}$cllARn$UZcAMm{KQ(_PTv8M%o~4<$gE{yPEB+X+akCZL)}z}nRaynaK#g~-I_ug>|{kI3jS z)gMN{l}4G$w4nG?+ygV@d`kpUSY=*>+dRji}!=Evf|9{ { + 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) + }) + }) +}) diff --git a/test/lib/browsers/base.ts b/test/lib/browsers/base.ts index bdebbfed6064a9c..e9abaf63aeb6196 100644 --- a/test/lib/browsers/base.ts +++ b/test/lib/browsers/base.ts @@ -59,7 +59,7 @@ export class BrowserInterface { deleteCookies(): BrowserInterface { return this } - async loadPage(url: string): Promise {} + async loadPage(url: string, { disableCache: boolean }): Promise {} async get(url: string): Promise {} async getValue(): Promise {} diff --git a/test/lib/browsers/playwright.ts b/test/lib/browsers/playwright.ts index 45e669226c911ab..419d865a1872d8a 100644 --- a/test/lib/browsers/playwright.ts +++ b/test/lib/browsers/playwright.ts @@ -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( @@ -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 diff --git a/test/lib/next-webdriver.ts b/test/lib/next-webdriver.ts index 73b056716de813d..a7321fcaf49e7b0 100644 --- a/test/lib/next-webdriver.ts +++ b/test/lib/next-webdriver.ts @@ -48,7 +48,8 @@ export default async function webdriver( appPortOrUrl: string | number, url: string, waitHydration = true, - retryWaitHydration = false + retryWaitHydration = false, + disableCache = false ): Promise { let CurrentInterface: typeof BrowserInterface @@ -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 diff --git a/yarn.lock b/yarn.lock index b376efc5417dbc5..8ee3d99053edc44 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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" @@ -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" @@ -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" @@ -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" @@ -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" @@ -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"