Skip to content

Commit

Permalink
Remove unnecessary <noscript> from next/future/image (#38080)
Browse files Browse the repository at this point in the history
We don't need to include `<noscript>` for `next/future/image` since it uses native lazy loading instead of the `IntersectionObserver` (#37927).

The only case when we still need `<noscript>` is for `placeholder="blur"` because it requires client-side JS to switch the from blur image to final image on load.
  • Loading branch information
styfle committed Jun 28, 2022
1 parent 52763de commit 0cb1253
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 27 deletions.
2 changes: 1 addition & 1 deletion packages/next/client/future/image.tsx
Expand Up @@ -677,7 +677,7 @@ const ImageElement = ({
}
}}
/>
{(isLazy || placeholder === 'blur') && (
{placeholder === 'blur' && (
<noscript>
<img
{...rest}
Expand Down
6 changes: 2 additions & 4 deletions test/integration/image-future/default/test/index.test.js
Expand Up @@ -169,11 +169,9 @@ function runTests(mode) {
const $html = cheerio.load(html)

const els = [].slice.apply($html('img'))
expect(els.length).toBe(2)
expect(els.length).toBe(1)

const [el, noscriptEl] = els
expect(noscriptEl.attribs.src).toBeDefined()
expect(noscriptEl.attribs.srcset).toBeDefined()
const [el] = els

expect(el.attribs.src).not.toBe('/truck.jpg')
expect(el.attribs.srcset).not.toBe(
Expand Down
10 changes: 9 additions & 1 deletion test/integration/image-future/noscript/pages/index.js
Expand Up @@ -9,14 +9,22 @@ const Page = () => {
return (
<div>
<p>noscript images</p>
<Image id="basic-image" src="/basic-image.jpg" width={640} height={360} />
<Image id="basic-image" src="/basic.jpg" width={640} height={360} />
<Image
loader={myLoader}
id="image-with-loader"
src="/remote-image.jpg"
width={640}
height={360}
/>
<Image
id="image-with-blur"
src="/blur.jpg"
width={640}
height={360}
placeholder="blur"
blurDataURL="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mMU/M9QDwADygGR4qH9qQAAAABJRU5ErkJggg=="
/>
</div>
)
}
Expand Down
54 changes: 33 additions & 21 deletions test/integration/image-future/noscript/test/index.test.js
Expand Up @@ -3,8 +3,9 @@
import { join } from 'path'
import cheerio from 'cheerio'
import {
killApp,
findPort,
killApp,
launchApp,
nextStart,
nextBuild,
renderViaHTTP,
Expand All @@ -14,29 +15,40 @@ const appDir = join(__dirname, '../')
let appPort
let app

describe('Noscript Tests', () => {
beforeAll(async () => {
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
function runTests() {
it('should include noscript for placeholder=blur but not others', async () => {
const html = await renderViaHTTP(appPort, '/')
const $ = cheerio.load(html)

expect($('noscript > img#basic-image').attr('src')).toBeUndefined()
expect($('noscript > img#image-with-loader').attr('src')).toBeUndefined()
expect($('noscript > img#image-with-blur').attr('src')).toMatch('blur.jpg')
})
afterAll(() => killApp(app))
describe('Noscript page source tests', () => {
it('should use local API for noscript img#basic-image src attribute', async () => {
const html = await renderViaHTTP(appPort, '/')
const $ = cheerio.load(html)

expect($('noscript > img#basic-image').attr('src')).toMatch(
/^\/_next\/image/
)
}

describe('Future Image Component Noscript Tests', () => {
describe('dev mode', () => {
beforeAll(async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort)
})
afterAll(async () => {
await killApp(app)
})
it('should use loader url for noscript img#image-with-loader src attribute', async () => {
const html = await renderViaHTTP(appPort, '/')
const $ = cheerio.load(html)

expect($('noscript > img#image-with-loader').attr('src')).toMatch(
/^https:\/\/example\.vercel\.sh/
)
runTests()
})

describe('server mode', () => {
beforeAll(async () => {
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
})
afterAll(async () => {
await killApp(app)
})

runTests()
})
})

0 comments on commit 0cb1253

Please sign in to comment.