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.

Sign up for GitHub

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

Jump to bottom

fix(next/script): bugs when used with/without <Head> #37747

Closed
wants to merge 4 commits into from
Closed

fix(next/script): bugs when used with/without <Head> #37747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f6997b1
test(next/script): failing `beforeInteractive` tests for bug #37741
aboqasem Jun 16, 2022
1e22020
revert: "test(next/script): failing `beforeInteractive` tests for bug…
aboqasem Jun 18, 2022
990b58a
test(next/script): failing tests for bug #37741
aboqasem Jun 18, 2022
981f9bc
fix(next/script): possible fix for `Script` in `next/head`
aboqasem Jun 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions packages/next/pages/_document.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -694,22 +694,24 @@ export class Head extends Component<HeadProps> {
let { head } = this.context
let cssPreloads: Array<JSX.Element> = []
let otherHeadElements: Array<JSX.Element> = []
let children = React.Children.toArray(this.props.children).filter(Boolean)
if (head) {
head.forEach((c) => {
if (
c &&
c.type === 'link' &&
c?.type === 'link' &&
c.props['rel'] === 'preload' &&
c.props['as'] === 'style'
) {
cssPreloads.push(c)
} else if (c?.type === Script) {
children.push(c)
} else {
c && otherHeadElements.push(c)
}
})
head = cssPreloads.concat(otherHeadElements)
}
let children = React.Children.toArray(this.props.children).filter(Boolean)

// show a warning if Head contains <title> (only in development)
if (process.env.NODE_ENV !== 'production') {
children = React.Children.map(children, (child: any) => {
Expand Down
36 changes: 31 additions & 5 deletions test/integration/script-loader/base/pages/_document.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,41 @@ export default function Document() {
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=scriptBeforeInteractive"
strategy="beforeInteractive"
></Script>

<Script
id="inline-before"
id="script-bi-inline-in-doc-in-head"
strategy="beforeInteractive"
dangerouslySetInnerHTML={{
__html: `console.log('inline beforeInteractive')`,
}}
></Script>
>{`console.log('script-bi-inline-in-doc-in-head')`}</Script>
<Script
id="script-ai-inline-in-doc-in-head"
strategy="afterInteractive"
>{`console.log('script-ai-inline-in-doc-in-head')`}</Script>
<Script
id="script-lo-inline-in-doc-in-head"
strategy="lazyOnload"
>{`console.log('script-lo-inline-in-doc-in-head')`}</Script>
{/* <Script
id="script-w-inline-in-doc-in-head"
strategy="worker"
>{`console.log('script-w-inline-in-doc-in-head')`}</Script> */}
</Head>
<body>
<Script
id="script-bi-inline-in-doc-out-head"
strategy="beforeInteractive"
>{`console.log('script-bi-inline-in-doc-out-head')`}</Script>
<Script
id="script-ai-inline-in-doc-out-head"
strategy="afterInteractive"
>{`console.log('script-ai-inline-in-doc-out-head')`}</Script>
<Script
id="script-lo-inline-in-doc-out-head"
strategy="lazyOnload"
>{`console.log('script-lo-inline-in-doc-out-head')`}</Script>
{/* <Script
id="script-w-inline-in-doc-out-head"
strategy="worker"
>{`console.log('script-w-inline-in-doc-out-head')`}</Script> */}
<Main />
<NextScript />
<div id="text" />
Expand Down
36 changes: 36 additions & 0 deletions test/integration/script-loader/base/pages/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,45 @@
import Script from 'next/script'
import Link from 'next/link'
import Head from 'next/head'

const Page = () => {
return (
<div class="container">
<Head>
<Script
id="script-bi-inline-in-page-in-head"
strategy="beforeInteractive"
>{`console.log('script-bi-inline-in-page-in-head')`}</Script>
<Script
id="script-ai-inline-in-page-in-head"
strategy="afterInteractive"
>{`console.log('script-ai-inline-in-page-in-head')`}</Script>
<Script
id="script-lo-inline-in-page-in-head"
strategy="lazyOnload"
>{`console.log('script-lo-inline-in-page-in-head')`}</Script>
{/* <Script
id="script-w-inline-in-page-in-head"
strategy="worker"
>{`console.log('script-w-inline-in-page-in-head')`}</Script> */}
</Head>
<Script
id="script-bi-inline-in-page-out-head"
strategy="beforeInteractive"
>{`console.log('script-bi-inline-in-page-out-head')`}</Script>
<Script
id="script-ai-inline-in-page-out-head"
strategy="afterInteractive"
>{`console.log('script-ai-inline-in-page-out-head')`}</Script>
<Script
id="script-lo-inline-in-page-out-head"
strategy="lazyOnload"
>{`console.log('script-lo-inline-in-page-out-head')`}</Script>
{/* <Script
id="script-w-inline-in-page-out-head"
strategy="worker"
>{`console.log('script-w-inline-in-page-out-head')`}</Script> */}

<Script
id="scriptAfterInteractive"
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=scriptAfterInteractive"
Expand Down
67 changes: 47 additions & 20 deletions test/integration/script-loader/test/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,50 @@ describe('Next.js Script - Primary Strategies', () => {
stopApp(server)
})

it('should execute scripts with any strategy in Document or Page inside or outside of Head', async () => {
const expected = {
'script-bi-inline-in-doc-in-head': true,
'script-ai-inline-in-doc-in-head': true,
'script-lo-inline-in-doc-in-head': true,
// 'script-w-inline-in-doc-in-head': true,
'script-bi-inline-in-doc-out-head': true,
'script-ai-inline-in-doc-out-head': true,
'script-lo-inline-in-doc-out-head': true,
// 'script-w-inline-in-doc-out-head': true,
'script-bi-inline-in-page-in-head': true,
'script-ai-inline-in-page-in-head': true,
'script-lo-inline-in-page-in-head': true,
// 'script-w-inline-in-page-in-head': true,
'script-bi-inline-in-page-out-head': true,
'script-ai-inline-in-page-out-head': true,
'script-lo-inline-in-page-out-head': true,
// 'script-w-inline-in-page-out-head': true,
}
const scripts = Object.keys(expected)

const actual = scripts.reduce((acc, script) => {
acc[script] = false
return acc
}, {})

let browser
try {
browser = await webdriver(appPort, '/')
await waitFor(1000)
const logs = await browser.log()

logs.forEach(({ message }) => {
if (message in actual) {
actual[message] = true
}
})

expect(actual).toEqual(expected)
} finally {
if (browser) await browser.close()
}
})

it('priority afterInteractive', async () => {
let browser
try {
Expand Down Expand Up @@ -194,33 +238,16 @@ describe('Next.js Script - Primary Strategies', () => {
const html = await renderViaHTTP(appPort, '/page5')
const $ = cheerio.load(html)

const script = $('#inline-before')
const script = $('#script-bi-inline-in-doc-in-head')
expect(script.length).toBe(1)

// Script is inserted before CSS
expect(
$(`#inline-before ~ link[href^="/_next/static/css"]`).length
$(`#script-bi-inline-in-doc-in-head ~ link[href^="/_next/static/css"]`)
.length
).toBeGreaterThan(0)
})

it('priority beforeInteractive with inline script should execute', async () => {
let browser
try {
browser = await webdriver(appPort, '/page7')

await waitFor(1000)

const logs = await browser.log()
expect(
logs.some((log) =>
log.message.includes('beforeInteractive inline script run')
)
).toBe(true)
} finally {
if (browser) await browser.close()
}
})

it('Does not duplicate inline scripts', async () => {
let browser
try {
Expand Down