Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary <noscript> from next/future/image #38080

Merged
merged 3 commits into from Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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()
})
})