fix: asset prefix on images #28306

ScriptedAlchemy · 2021-08-19T20:16:35Z

Adding asset prefix support to next-images-loader
fixes #28305

Bug

Related issues linked using fixes #number
Integration tests added
Errors have helpful link attached, see contributing.md

Feature

Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
Related issues linked using fixes #number
Integration tests added
Documentation added
Telemetry added. In case of a feature if it's used or not.
Errors have helpful link attached, see contributing.md

Documentation / Examples

Make sure the linting passes

Adding asset prefix support to next-images-loader

ijjk · 2021-08-19T20:31:11Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
buildDuration	17.2s	17.5s	⚠️ +302ms
buildDurationCached	4.2s	4.3s	⚠️ +69ms
nodeModulesSize	61.4 MB	61.4 MB	⚠️ +200 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.364	3.37	⚠️ +0.01
/ avg req/sec	743.18	741.75	⚠️ -1.43
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	2.252	2.226	-0.03
/error-in-render avg req/sec	1110.24	1123.23	+12.99

Client Bundles (main, webpack, commons)

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
745.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	23.2 kB	23.2 kB	✓
webpack-HASH.js gzip	1.45 kB	1.45 kB	✓
Overall change	67.1 kB	67.1 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
polyfills-a4..dd70.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
_app-HASH.js gzip	979 B	979 B	✓
_error-HASH.js gzip	194 B	194 B	✓
amp-HASH.js gzip	312 B	312 B	✓
css-HASH.js gzip	329 B	329 B	✓
dynamic-HASH.js gzip	2.67 kB	2.67 kB	✓
head-HASH.js gzip	351 B	351 B	✓
hooks-HASH.js gzip	918 B	918 B	✓
image-HASH.js gzip	4.14 kB	4.14 kB	✓
index-HASH.js gzip	261 B	261 B	✓
link-HASH.js gzip	1.66 kB	1.66 kB	✓
routerDirect..HASH.js gzip	318 B	318 B	✓
script-HASH.js gzip	387 B	387 B	✓
withRouter-HASH.js gzip	320 B	320 B	✓
bb14e60e810b..30f.css gzip	125 B	125 B	✓
Overall change	13 kB	13 kB	✓

Client Build Manifests

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
_buildManifest.js gzip	491 B	491 B	✓
Overall change	491 B	491 B	✓

Rendered Page Sizes

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
index.html gzip	541 B	541 B	✓
link.html gzip	553 B	553 B	✓
withRouter.html gzip	534 B	534 B	✓
Overall change	1.63 kB	1.63 kB	✓

Webpack 4 Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
buildDuration	14.1s	14.2s	⚠️ +96ms
buildDurationCached	5.9s	5.9s	-25ms
nodeModulesSize	61.4 MB	61.4 MB	⚠️ +200 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.442	3.321	-0.12
/ avg req/sec	726.42	752.72	+26.3
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	2.263	2.146	-0.12
/error-in-render avg req/sec	1104.89	1165.23	+60.34

Client Bundles (main, webpack, commons)

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
16.HASH.js gzip	186 B	186 B	✓
677f882d2ed8..HASH.js gzip	14.1 kB	14.1 kB	✓
framework.HASH.js gzip	41.9 kB	41.9 kB	✓
main-HASH.js gzip	10.6 kB	10.6 kB	✓
webpack-HASH.js gzip	1.19 kB	1.19 kB	✓
Overall change	68 kB	68 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
polyfills-a4..dd70.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
_app-HASH.js gzip	964 B	964 B	✓
_error-HASH.js gzip	3.8 kB	3.8 kB	✓
amp-HASH.js gzip	552 B	552 B	✓
css-HASH.js gzip	333 B	333 B	✓
dynamic-HASH.js gzip	2.87 kB	2.87 kB	✓
head-HASH.js gzip	3.06 kB	3.06 kB	✓
hooks-HASH.js gzip	924 B	924 B	✓
index-HASH.js gzip	231 B	231 B	✓
link-HASH.js gzip	1.64 kB	1.64 kB	✓
routerDirect..HASH.js gzip	298 B	298 B	✓
script-HASH.js gzip	2.98 kB	2.98 kB	✓
withRouter-HASH.js gzip	295 B	295 B	✓
30809af5c834..565.css gzip	125 B	125 B	✓
Overall change	18.1 kB	18.1 kB	✓

Client Build Manifests

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
_buildManifest.js gzip	500 B	500 B	✓
Overall change	500 B	500 B	✓

Rendered Page Sizes

	vercel/next.js canary	ScriptedAlchemy/next.js patch-1	Change
index.html gzip	585 B	585 B	✓
link.html gzip	597 B	597 B	✓
withRouter.html gzip	577 B	577 B	✓
Overall change	1.76 kB	1.76 kB	✓

Commit: fb0f894

ijjk · 2021-08-19T20:41:03Z

Failing test suites

Commit: fb0f894

test/integration/image-component/asset-prefix/test/index.test.js

Image Component assetPrefix Tests > dev mode > should include assetPrefix when placeholder=blur during next dev

Expand output

● Image Component assetPrefix Tests › dev mode › should include assetPrefix when placeholder=blur during next dev

JavascriptError: javascript error: Cannot read property 'style' of null
  (Session info: headless chrome=92.0.4515.131)

  31 |         browser = await webdriver(appPort, '/')
  32 |         const id = 'test1'
> 33 |         const bgImage = await browser.eval(
     |                         ^
  34 |           `document.getElementById('${id}').style['background-image']`
  35 |         )
  36 |         expect(bgImage).toMatch(

  at Object.throwDecodedError (../node_modules/selenium-webdriver/lib/error.js:550:15)
  at parseHttpResponse (../node_modules/selenium-webdriver/lib/http.js:565:13)
  at Executor.execute (../node_modules/selenium-webdriver/lib/http.js:491:26)
  at thenableWebDriverProxy.execute (../node_modules/selenium-webdriver/lib/webdriver.js:700:17)
  at Object.<anonymous> (integration/image-component/asset-prefix/test/index.test.js:33:25)

styfle

Thanks for the PR!

Please add a test to ensure this change works correctly, thanks!

styfle · 2021-08-19T21:40:09Z

There's actually an existing test fixture here and this PR made it fail https://github.com/vercel/next.js/tree/canary/test/integration/image-component/asset-prefix

ScriptedAlchemy · 2021-08-31T05:39:59Z

I'll see if I can find time to create a test. Already extracted it from next and rebuilt it as a custom loader

timneutkens

assetPrefix is intentionally not added to the start of the url, same as next/image. Instead of assetPrefix it should likely be basePath in this case. In particular you can use images.path which already includes /_next/image with the basePath

styfle · 2021-09-28T13:14:51Z

Thanks! This was fixed in #29307.

fix: asset prefix on images

fb0f894

Adding asset prefix support to next-images-loader

ScriptedAlchemy requested review from huozhi, ijjk, padmaia, shuding, styfle and timneutkens as code owners August 19, 2021 20:16

ijjk added the type: next label Aug 19, 2021

ScriptedAlchemy mentioned this pull request Aug 19, 2021

next-image-loader does not respect assetPrefix #28305

Closed

styfle reviewed Aug 19, 2021

View reviewed changes

timneutkens requested changes Aug 31, 2021

View reviewed changes

styfle closed this Sep 28, 2021

vercel locked as resolved and limited conversation to collaborators Jan 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: asset prefix on images #28306

fix: asset prefix on images #28306

ScriptedAlchemy commented Aug 19, 2021

ijjk commented Aug 19, 2021

ijjk commented Aug 19, 2021

styfle left a comment

styfle commented Aug 19, 2021 •

edited

Loading

ScriptedAlchemy commented Aug 31, 2021

timneutkens left a comment •

edited

Loading

styfle commented Sep 28, 2021

fix: asset prefix on images #28306

fix: asset prefix on images #28306

Conversation

ScriptedAlchemy commented Aug 19, 2021

Bug

Feature

Documentation / Examples

ijjk commented Aug 19, 2021

Stats from current PR

ijjk commented Aug 19, 2021

Failing test suites

styfle left a comment

Choose a reason for hiding this comment

styfle commented Aug 19, 2021 • edited Loading

ScriptedAlchemy commented Aug 31, 2021

timneutkens left a comment • edited Loading

Choose a reason for hiding this comment

styfle commented Sep 28, 2021

styfle commented Aug 19, 2021 •

edited

Loading

timneutkens left a comment •

edited

Loading