Skip to content

Commit

Permalink
Only warn styles and scripts under next head in concurrent mode (#34897)
Browse files Browse the repository at this point in the history
x-ref: #34021 , #34004

Only log each warning once and only trigger in concurrent mode


## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`
  • Loading branch information
huozhi committed Feb 28, 2022
1 parent 8e3b6fc commit 57702cb
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 88 deletions.
12 changes: 1 addition & 11 deletions packages/next/client/image.tsx
Expand Up @@ -8,6 +8,7 @@ import {
} from '../shared/lib/image-config'
import { useIntersection } from './use-intersection'
import { ImageConfigContext } from '../shared/lib/image-config-context'
import { warnOnce } from '../shared/lib/utils'

const configEnv = process.env.__NEXT_IMAGE_OPTS as any as ImageConfigComplete
const loadedImageURLs = new Set<string>()
Expand All @@ -23,17 +24,6 @@ if (typeof window === 'undefined') {
;(global as any).__NEXT_IMAGE_IMPORTED = true
}

let warnOnce = (_: string) => {}
if (process.env.NODE_ENV !== 'production') {
const warnings = new Set<string>()
warnOnce = (msg: string) => {
if (!warnings.has(msg)) {
console.warn(msg)
}
warnings.add(msg)
}
}

const VALID_LOADING_VALUES = ['lazy', 'eager', undefined] as const
type LoadingValue = typeof VALID_LOADING_VALUES[number]
type ImageConfig = ImageConfigComplete & { allSizes: number[] }
Expand Down
10 changes: 7 additions & 3 deletions packages/next/shared/lib/head.tsx
Expand Up @@ -3,6 +3,7 @@ import Effect from './side-effect'
import { AmpStateContext } from './amp-context'
import { HeadManagerContext } from './head-manager-context'
import { isInAmpMode } from './amp'
import { warnOnce } from './utils'

type WithInAmpMode = {
inAmpMode?: boolean
Expand Down Expand Up @@ -161,17 +162,20 @@ function reduceComponents(
return React.cloneElement(c, newProps)
}
}
if (process.env.NODE_ENV === 'development') {
if (
process.env.NODE_ENV === 'development' &&
process.env.__NEXT_CONCURRENT_FEATURES
) {
// omit JSON-LD structured data snippets from the warning
if (c.type === 'script' && c.props['type'] !== 'application/ld+json') {
const srcMessage = c.props['src']
? `<script> tag with src="${c.props['src']}"`
: `inline <script>`
console.warn(
warnOnce(
`Do not add <script> tags using next/head (see ${srcMessage}). Use next/script instead. \nSee more info here: https://nextjs.org/docs/messages/no-script-tags-in-head-component`
)
} else if (c.type === 'link' && c.props['rel'] === 'stylesheet') {
console.warn(
warnOnce(
`Do not add stylesheets using next/head (see <link rel="stylesheet"> tag with href="${c.props['href']}"). Use Document instead. \nSee more info here: https://nextjs.org/docs/messages/no-stylesheets-in-head-component`
)
}
Expand Down
13 changes: 13 additions & 0 deletions packages/next/shared/lib/utils.ts
Expand Up @@ -369,6 +369,19 @@ export async function loadGetInitialProps<
return props
}

let warnOnce = (_: string) => {}
if (process.env.NODE_ENV !== 'production') {
const warnings = new Set<string>()
warnOnce = (msg: string) => {
if (!warnings.has(msg)) {
console.warn(msg)
}
warnings.add(msg)
}
}

export { warnOnce }

export const SP = typeof performance !== 'undefined'
export const ST =
SP &&
Expand Down
69 changes: 0 additions & 69 deletions test/integration/client-navigation/test/index.test.js
Expand Up @@ -1430,75 +1430,6 @@ describe('Client Navigation', () => {
}
})

it('should warn when scripts are in head', async () => {
let browser
try {
browser = await webdriver(context.appPort, '/head')

await browser.waitForElementByCss('h1')
await waitFor(2000)
const browserLogs = await browser.log('browser')
let found = false
browserLogs.forEach((log) => {
console.log('log.message', log.message)
if (log.message.includes('Use next/script instead')) {
found = true
}
})
expect(found).toEqual(true)
} finally {
if (browser) {
await browser.close()
}
}
})

it('should not warn when application/ld+json scripts are in head', async () => {
let browser
try {
browser = await webdriver(context.appPort, '/head-with-json-ld-snippet')

await browser.waitForElementByCss('h1')
await waitFor(2000)
const browserLogs = await browser.log('browser')
let found = false
browserLogs.forEach((log) => {
console.log('log.message', log.message)
if (log.message.includes('Use next/script instead')) {
found = true
}
})
expect(found).toEqual(false)
} finally {
if (browser) {
await browser.close()
}
}
})

it('should warn when stylesheets are in head', async () => {
let browser
try {
browser = await webdriver(context.appPort, '/head')

await browser.waitForElementByCss('h1')
await waitFor(2000)
const browserLogs = await browser.log('browser')
let found = false
browserLogs.forEach((log) => {
console.log('log.message', log.message)
if (log.message.includes('Do not add stylesheets using next/head')) {
found = true
}
})
expect(found).toEqual(true)
} finally {
if (browser) {
await browser.close()
}
}
})

it('should update head during client routing', async () => {
let browser
try {
Expand Down
@@ -0,0 +1,15 @@
import React from 'react'
import Head from 'next/head'

export default () => (
<div>
<Head>
<link rel="stylesheet" href="style-a.css" key="my-style" />
<link rel="stylesheet" href="style-b.css" key="my-style" />

<script src="/test-async.js" async></script>
<script src="/test-defer.js" defer="yas"></script>
</Head>
<h1>Streaming Head</h1>
</div>
)
Expand Up @@ -151,7 +151,7 @@ describe('Edge runtime - prod', () => {
})

basic(context, { env: 'prod' })
streaming(context)
streaming(context, { env: 'prod' })
rsc(context, { runtime: 'edge', env: 'prod' })
})

Expand Down Expand Up @@ -184,14 +184,14 @@ describe('Edge runtime - dev', () => {
})

basic(context, { env: 'dev' })
streaming(context)
streaming(context, { env: 'dev' })
rsc(context, { runtime: 'edge', env: 'dev' })
})

const nodejsRuntimeBasicSuite = {
runTests: (context, env) => {
basic(context, { env })
streaming(context)
streaming(context, { env })
rsc(context, { runtime: 'nodejs' })

if (env === 'prod') {
Expand Down
@@ -1,6 +1,6 @@
/* eslint-env jest */
import webdriver from 'next-webdriver'
import { fetchViaHTTP } from 'next-test-utils'
import { fetchViaHTTP, waitFor } from 'next-test-utils'

async function resolveStreamResponse(response, onData) {
let result = ''
Expand All @@ -16,7 +16,7 @@ async function resolveStreamResponse(response, onData) {
return result
}

export default function (context) {
export default function (context, { env }) {
it('should support streaming for fizz response', async () => {
await fetchViaHTTP(context.appPort, '/streaming', null, {}).then(
async (response) => {
Expand Down Expand Up @@ -99,4 +99,82 @@ export default function (context) {
expect(result).toMatch(/<\/body><\/html>/)
})
})

if (env === 'dev') {
it('should warn when stylesheets or scripts are in head', async () => {
let browser
try {
browser = await webdriver(context.appPort, '/head')

await browser.waitForElementByCss('h1')
await waitFor(1000)
const browserLogs = await browser.log('browser')
let foundStyles = false
let foundScripts = false
const logs = []
browserLogs.forEach(({ message }) => {
if (message.includes('Do not add stylesheets using next/head')) {
foundStyles = true
logs.push(message)
}
if (message.includes('Do not add <script> tags using next/head')) {
foundScripts = true
logs.push(message)
}
})

expect(foundStyles).toEqual(true)
expect(foundScripts).toEqual(true)

// Warnings are unique
expect(logs.length).toEqual(new Set(logs).size)
} finally {
if (browser) {
await browser.close()
}
}
})

it('should warn when scripts are in head', async () => {
let browser
try {
browser = await webdriver(context.appPort, '/head')
await browser.waitForElementByCss('h1')
await waitFor(1000)
const browserLogs = await browser.log('browser')
let found = false
browserLogs.forEach((log) => {
if (log.message.includes('Use next/script instead')) {
found = true
}
})
expect(found).toEqual(true)
} finally {
if (browser) {
await browser.close()
}
}
})

it('should not warn when application/ld+json scripts are in head', async () => {
let browser
try {
browser = await webdriver(context.appPort, '/head-with-json-ld-snippet')
await browser.waitForElementByCss('h1')
await waitFor(1000)
const browserLogs = await browser.log('browser')
let found = false
browserLogs.forEach((log) => {
if (log.message.includes('Use next/script instead')) {
found = true
}
})
expect(found).toEqual(false)
} finally {
if (browser) {
await browser.close()
}
}
})
}
}

0 comments on commit 57702cb

Please sign in to comment.