Skip to content

Commit

Permalink
Adds inline script functionality to next/script for worker and `b…
Browse files Browse the repository at this point in the history
…eforeInteractive` strategies (#36364)

Adds inline script functionality to `next/script` for `worker` and `beforeInteractive` strategies. 

- fixes #36318 
- fixes #26343
- fixes #26591
- fixes #26343
- fixes #26240


Co-authored-by: Janicklas Ralph <6142074+janicklas-ralph@users.noreply.github.com>
  • Loading branch information
housseindjirdeh and janicklas-ralph committed Apr 29, 2022
1 parent e30d723 commit fd2ba11
Show file tree
Hide file tree
Showing 8 changed files with 248 additions and 15 deletions.
5 changes: 1 addition & 4 deletions docs/basic-features/script.md
Expand Up @@ -243,10 +243,7 @@ Or by using the `dangerouslySetInnerHTML` property:
/>
```

There are two limitations to be aware of when using the Script component for inline scripts:

- Only the `afterInteractive` and `lazyOnload` strategies can be used. The `beforeInteractive` loading strategy injects the contents of an external script into the initial HTML response. Inline scripts already do this, which is why **the `beforeInteractive` strategy cannot be used with inline scripts.**
- An `id` attribute must be defined in order for Next.js to track and optimize the script
The `id` property is required for **inline scripts** in order for Next.js to track and optimize the script.

### Executing Code After Loading (`onLoad`)

Expand Down
38 changes: 38 additions & 0 deletions errors/invalid-script.md
@@ -0,0 +1,38 @@
# Invalid Script

#### Why This Error Occurred

Somewhere in your application, you are using the `next/script` component without including an inline script or `src` attribute.

#### Possible Ways to Fix It

Look for any usage of the `next/script` component and make sure that `src` is provided or an inline script is used.

**Compatible `src` attribute**

```jsx
<Script src="https://example.com/analytics.js" />
```

**Compatible inline script with curly braces**

```jsx
<Script id="show-banner">
{`document.getElementById('banner').classList.remove('hidden')`}
</Script>
```

**Compatible inline script with `dangerouslySetInnerHtml`**

```jsx
<Script
id="show-banner"
dangerouslySetInnerHTML={{
__html: `document.getElementById('banner').classList.remove('hidden')`,
}}
/>
```

### Useful Links

- [Script Component in Documentation](https://nextjs.org/docs/basic-features/script)
4 changes: 4 additions & 0 deletions errors/manifest.json
Expand Up @@ -657,6 +657,10 @@
{
"title": "no-assign-module-variable",
"path": "/errors/no-assign-module-variable.md"
},
{
"title": "invalid-script",
"path": "/errors/invalid-script.md"
}
]
}
Expand Down
1 change: 0 additions & 1 deletion packages/next/client/script.tsx
Expand Up @@ -149,7 +149,6 @@ function Script(props: ScriptProps): JSX.Element | null {
const {
src = '',
onLoad = () => {},
dangerouslySetInnerHTML,
strategy = 'afterInteractive',
onError,
...restProps
Expand Down
97 changes: 88 additions & 9 deletions packages/next/pages/_document.tsx
Expand Up @@ -121,12 +121,54 @@ function getPreNextWorkerScripts(context: HtmlProps, props: OriginProps) {
}}
/>
{(scriptLoader.worker || []).map((file: ScriptProps, index: number) => {
const { strategy, ...scriptProps } = file
const {
strategy,
src,
children: scriptChildren,
dangerouslySetInnerHTML,
...scriptProps
} = file

let srcProps: {
src?: string
dangerouslySetInnerHTML?: {
__html: string
}
} = {}

if (src) {
// Use external src if provided
srcProps.src = src
} else if (
dangerouslySetInnerHTML &&
dangerouslySetInnerHTML.__html
) {
// Embed inline script if provided with dangerouslySetInnerHTML
srcProps.dangerouslySetInnerHTML = {
__html: dangerouslySetInnerHTML.__html,
}
} else if (scriptChildren) {
// Embed inline script if provided with children
srcProps.dangerouslySetInnerHTML = {
__html:
typeof scriptChildren === 'string'
? scriptChildren
: Array.isArray(scriptChildren)
? scriptChildren.join('')
: '',
}
} else {
throw new Error(
'Invalid usage of next/script. Did you forget to include a src attribute or an inline script? https://nextjs.org/docs/messages/invalid-script'
)
}

return (
<script
{...srcProps}
{...scriptProps}
type="text/partytown"
key={scriptProps.src || index}
key={src || index}
nonce={props.nonce}
data-nscript="worker"
crossOrigin={props.crossOrigin || crossOrigin}
Expand All @@ -137,9 +179,7 @@ function getPreNextWorkerScripts(context: HtmlProps, props: OriginProps) {
)
} catch (err) {
if (isError(err) && err.code !== 'MODULE_NOT_FOUND') {
console.warn(
`Warning: Partytown could not be instantiated in your application due to an error. ${err.message}`
)
console.warn(`Warning: ${err.message}`)
}
return null
}
Expand All @@ -150,8 +190,9 @@ function getPreNextScripts(context: HtmlProps, props: OriginProps) {

const webWorkerScripts = getPreNextWorkerScripts(context, props)

const beforeInteractiveScripts = (scriptLoader.beforeInteractive || []).map(
(file: ScriptProps, index: number) => {
const beforeInteractiveScripts = (scriptLoader.beforeInteractive || [])
.filter((script) => script.src)
.map((file: ScriptProps, index: number) => {
const { strategy, ...scriptProps } = file
return (
<script
Expand All @@ -163,8 +204,7 @@ function getPreNextScripts(context: HtmlProps, props: OriginProps) {
crossOrigin={props.crossOrigin || crossOrigin}
/>
)
}
)
})

return (
<>
Expand Down Expand Up @@ -500,6 +540,44 @@ export class Head extends Component<
]
}

getBeforeInteractiveInlineScripts() {
const { scriptLoader } = this.context
const { nonce, crossOrigin } = this.props

return (scriptLoader.beforeInteractive || [])
.filter(
(script) =>
!script.src && (script.dangerouslySetInnerHTML || script.children)
)
.map((file: ScriptProps, index: number) => {
const { strategy, children, dangerouslySetInnerHTML, ...scriptProps } =
file
let html = ''

if (dangerouslySetInnerHTML && dangerouslySetInnerHTML.__html) {
html = dangerouslySetInnerHTML.__html
} else if (children) {
html =
typeof children === 'string'
? children
: Array.isArray(children)
? children.join('')
: ''
}

return (
<script
{...scriptProps}
dangerouslySetInnerHTML={{ __html: html }}
key={scriptProps.id || index}
nonce={nonce}
data-nscript="beforeInteractive"
crossOrigin={crossOrigin || process.env.__NEXT_CROSS_ORIGIN}
/>
)
})
}

getDynamicChunks(files: DocumentFiles) {
return getDynamicChunks(this.context, this.props, files)
}
Expand Down Expand Up @@ -782,6 +860,7 @@ export class Head extends Component<
href={canonicalBase + getAmpPath(ampPath, dangerousAsPath)}
/>
)}
{this.getBeforeInteractiveInlineScripts()}
{!optimizeCss && this.getCssLinks(files)}
{!optimizeCss && <noscript data-n-css={this.props.nonce ?? ''} />}

Expand Down
98 changes: 97 additions & 1 deletion test/e2e/next-script-worker-strategy/index.test.ts
Expand Up @@ -51,7 +51,7 @@ describe('experimental.nextScriptWorkers: false with no Partytown dependency', (
})
})

describe('experimental.nextScriptWorkers: true with required Partytown dependency', () => {
describe('experimental.nextScriptWorkers: true with required Partytown dependency for external script', () => {
let next: NextInstance

beforeAll(async () => {
Expand Down Expand Up @@ -139,6 +139,102 @@ describe('experimental.nextScriptWorkers: true with required Partytown dependenc
})
})

describe('experimental.nextScriptWorkers: true with required Partytown dependency for inline script', () => {
const createNextApp = async (script) =>
await createNext({
nextConfig: {
experimental: {
nextScriptWorkers: true,
},
dependencies: {
react: '17',
'react-dom': '17',
},
},
files: {
'pages/index.js': `
import Script from 'next/script'
export default function Page() {
return (
<>
${script}
<div id="text" />
</>
)
}
`,
},
dependencies: {
'@builder.io/partytown': '0.4.2',
},
})

it('Inline worker script through children is modified by Partytown to execute on a worker thread', async () => {
let next: NextInstance
let browser: BrowserInterface

next = await createNextApp(
`<Script id="inline-script" strategy="worker">{"document.getElementById('text').textContent += 'abc'"}</Script>`
)

try {
browser = await webdriver(next.url, '/')

const predefinedWorkerScripts = await browser.eval(
`document.querySelectorAll('script[type="text/partytown"]').length`
)
expect(predefinedWorkerScripts).toEqual(1)

await waitFor(1000)

// Partytown modifes type to "text/partytown-x" after it has been executed in the web worker
const processedWorkerScripts = await browser.eval(
`document.querySelectorAll('script[type="text/partytown-x"]').length`
)
expect(processedWorkerScripts).toEqual(1)

const text = await browser.elementById('text').text()
expect(text).toBe('abc')
} finally {
if (browser) await browser.close()
await next.destroy()
}
})

it('Inline worker script through dangerouslySetInnerHtml is modified by Partytown to execute on a worker thread', async () => {
let next: NextInstance
let browser: BrowserInterface

next = await createNextApp(
`<Script id="inline-script" strategy="worker" dangerouslySetInnerHTML={{__html: "document.getElementById('text').textContent += 'abcd'"}}/>`
)

try {
browser = await webdriver(next.url, '/')

const predefinedWorkerScripts = await browser.eval(
`document.querySelectorAll('script[type="text/partytown"]').length`
)
expect(predefinedWorkerScripts).toEqual(1)

await waitFor(1000)

// Partytown modifes type to "text/partytown-x" after it has been executed in the web worker
const processedWorkerScripts = await browser.eval(
`document.querySelectorAll('script[type="text/partytown-x"]').length`
)
expect(processedWorkerScripts).toEqual(1)

const text = await browser.elementById('text').text()
expect(text).toBe('abcd')
} finally {
if (browser) await browser.close()
await next.destroy()
}
})
})

describe('experimental.nextScriptWorkers: true with config override', () => {
let next: NextInstance

Expand Down
7 changes: 7 additions & 0 deletions test/integration/script-loader/base/pages/_document.js
Expand Up @@ -16,6 +16,13 @@ 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"
strategy="beforeInteractive"
dangerouslySetInnerHTML={{
__html: `console.log('inline beforeInteractive')`,
}}
></Script>
</Head>
<body>
<Main />
Expand Down
13 changes: 13 additions & 0 deletions test/integration/script-loader/test/index.test.js
Expand Up @@ -190,6 +190,19 @@ describe('Next.js Script - Primary Strategies', () => {
}
})

it('priority beforeInteractive with inline script', async () => {
const html = await renderViaHTTP(appPort, '/page5')
const $ = cheerio.load(html)

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

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

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

0 comments on commit fd2ba11

Please sign in to comment.