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

Only warn styles and scripts under next head in concurrent mode #34897

Merged
merged 2 commits into from Feb 28, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
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()
}
}
})
}
}