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

Fix inline scripts being duplicated when used with next/script component #27218

Merged
merged 4 commits into from
Jul 16, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
5 changes: 4 additions & 1 deletion docs/basic-features/script.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ With `next/script`, you can define the `strategy` property and Next.js will o
- `afterInteractive` (**default**): For scripts that can fetch and execute **after** the page is interactive, such as tag managers and analytics. These scripts are injected on the client-side and will run after hydration.
- `lazyOnload` For scripts that can wait to load during idle time, such as chat support and social media widgets.

> **Note:** These loading strategies work the same for inline scripts wrapped with `<Script>`. See the inline scripts example below.
> **Note:**
>
> - `<Script>` supports inline scripts with `afterInteractive` and `lazyOnload` strategy
> - Inline scripts wrapped with `<Script>` _require an `id` attribute to be defined_. This helps to efficiently track and optimize the inline scripts.
timneutkens marked this conversation as resolved.
Show resolved Hide resolved

## Usage

Expand Down
17 changes: 11 additions & 6 deletions packages/next/client/script.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,17 @@ const loadScript = (props: ScriptProps): void => {
} = props

const cacheKey = id || src

// Script has already loaded
if (cacheKey && LoadCache.has(cacheKey)) {
return
}

// Contents of this script are already loading/loaded
if (ScriptCache.has(src)) {
if (!LoadCache.has(cacheKey)) {
LoadCache.add(cacheKey)
// Execute onLoad since the script loading has begun
ScriptCache.get(src).then(onLoad, onError)
}
LoadCache.add(cacheKey)
// Execute onLoad since the script loading has begun
ScriptCache.get(src).then(onLoad, onError)
return
}

Expand All @@ -67,8 +72,8 @@ const loadScript = (props: ScriptProps): void => {

if (src) {
ScriptCache.set(src, loadPromise)
LoadCache.add(cacheKey)
}
LoadCache.add(cacheKey)

if (dangerouslySetInnerHTML) {
el.innerHTML = dangerouslySetInnerHTML.__html || ''
Expand Down
7 changes: 6 additions & 1 deletion test/integration/script-loader/pages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ const Page = () => {
src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=scriptAfterInteractive"
></Script>
<div>index</div>
<Link href="/page1">Page1</Link>
<div>
<Link href="/page1">Page1</Link>
</div>
<div>
<Link href="/page5">Page5</Link>
</div>
</div>
)
}
Expand Down
16 changes: 16 additions & 0 deletions test/integration/script-loader/pages/page5.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import Link from 'next/link'
import Script from 'next/script'

const Page = () => {
return (
<div class="container">
<Script id="inline-script">{`document.getElementById('text').textContent += 'abc'`}</Script>
<div>page5</div>
<div>
<Link href="/">Index</Link>
</div>
</div>
)
}

export default Page
26 changes: 25 additions & 1 deletion test/integration/script-loader/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ describe('Script Loader', () => {
browser = await webdriver(appPort, '/')

await browser.waitForElementByCss('[href="/page1"]')
await browser.click('#page1')
await browser.click('[href="/page1"]')

await browser.waitForElementByCss('.container')
await waitFor(1000)
Expand All @@ -143,4 +143,28 @@ describe('Script Loader', () => {
if (browser) await browser.close()
}
})

it('Does not duplicate inline scripts', async () => {
let browser
try {
browser = await webdriver(appPort, '/')

// Navigate away and back to page
await browser.waitForElementByCss('[href="/page5"]')
await browser.click('[href="/page5"]')
await browser.waitForElementByCss('[href="/"]')
await browser.click('[href="/"]')
await browser.waitForElementByCss('[href="/page5"]')
await browser.click('[href="/page5"]')

await browser.waitForElementByCss('.container')
await waitFor(1000)

const text = await browser.elementById('text').text()

expect(text).toBe('abc')
} finally {
if (browser) await browser.close()
}
})
})