Skip to content

Commit

Permalink
feat(gatsby,gatsby-script): Do not use context for script collection (#…
Browse files Browse the repository at this point in the history
…36246)

* feat(gatsby): Do not use context for SSR script collection

* Update snapshots

* Fix location context

* Refactor global struct

* Make browser runtime work

* Only inject if scripts collected

* Refactor into separate modules

* Snapshots again

* Remove unit tests already covered in e2e

* Fix browser runtime location provider

* Add off-main-thread CSR navigation develop runtime tests

* Make comments more terse

* Duplicate tests to prod runtime

* Reorder assertions

* See if timeout fixes assertion failing only in CI

* Bump expect timeout in config instead

* Implement waitForRouteChange fixture to fix flakes

* Try strange wait pattern

* Sigh misconfig

* Skip CSR nav test, it's not worth it

* Update packages/gatsby/src/internal-plugins/partytown/gatsby-browser.tsx

Co-authored-by: Ward Peeters <ward@coding-tech.com>

* Cleanup

* PR comments

* Fix multiple service worker leak

* Remove off-main-thread CSR hack

* Update limitations in docs

* Update snapshot

* Guard for develop env

Co-authored-by: Ward Peeters <ward@coding-tech.com>
  • Loading branch information
tyhopp and wardpeet authored Aug 3, 2022
1 parent 8b37425 commit 49cf094
Show file tree
Hide file tree
Showing 25 changed files with 384 additions and 298 deletions.
5 changes: 4 additions & 1 deletion docs/docs/reference/built-in-components/gatsby-script.md
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,10 @@ You may need to adjust your dev tools to the verbose log level in order to see t

By leveraging [Partytown](https://partytown.builder.io), scripts that use the `off-main-thread` strategy must also be aware of the [limitations mentioned in the Partytown documentation](https://partytown.builder.io/trade-offs). While the strategy can be powerful, it may not be the best solution for all scenarios.

In addition, the `off-main-thread` strategy does not support the `onLoad` and `onError` callbacks.
In addition, there are other limitations that require upstream changes from Partytown to enable:

- The `onLoad` and `onError` callbacks are not supported. See [discussion #199 in the Partytown repo](https://github.com/BuilderIO/partytown/discussions/199).
- Scripts load only on server-side rendering (SSR) navigation (e.g. regular `<a>` tag navigation), and not on client-side rendering (CSR) navigation (e.g. Gatsby `<Link>` navigation). See [issue #74 in the Partytown repo](https://github.com/BuilderIO/partytown/issues/74).

## Usage in Gatsby SSR and Browser APIs

Expand Down
5 changes: 4 additions & 1 deletion e2e-tests/development-runtime/gatsby-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,8 @@ module.exports = {
// To learn more, visit: https://gatsby.dev/offline
// 'gatsby-plugin-offline',
],
partytownProxiedURLs: [`https://unpkg.com/three@0.139.1/build/three.js`],
partytownProxiedURLs: [
`https://unpkg.com/three@0.139.1/build/three.js`,
`http://localhost:8000/used-by-off-main-thread-2.js`,
],
}
5 changes: 5 additions & 0 deletions e2e-tests/development-runtime/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ const config: PlaywrightTestConfig = {
reporter: "html",
/* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
use: {
// Use to slow down for debugging
// launchOptions: {
// slowMo: 5 * 1000,
// },
headless: !!process.env.CI,
/* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */
actionTimeout: 0,
/* Base URL to use in actions like `await page.goto('/')`. */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,65 @@
import { test, expect } from "@playwright/test"

const id = {
templateLiteral: `inline-script-template-literal-mutation`,
dangerouslySet: `inline-script-dangerously-set-mutation`,
// Acutual <Script>
three: `three`,

// DOM nodes appended as a result of `<Script>` execution
templateLiteral: `mutation-inline-script-template-literal`,
dangerouslySet: `mutation-inline-script-dangerously-set`,
scriptWithSrc: `mutation-script-with-src`,
}

test.describe(`off-main-thread scripts`, () => {
test(`should load successfully`, async ({ page }) => {
test(`should load successfully when the page is visited directly`, async ({
page,
}) => {
await page.goto(`/gatsby-script-off-main-thread/`)

// @ts-ignore
const scriptWithSrc = await page.evaluate(() => typeof THREE === `function`)
const partytownSnippet = page.locator(`[data-partytown]`)
const scriptWithSrc = page.locator(`[id=${id.three}]`)
const templateLiteral = page.locator(`[id=${id.templateLiteral}]`)
const dangerouslySet = page.locator(`[id=${id.dangerouslySet}]`)

await expect(partytownSnippet).toContainText(/THREE/) // Forwards configured
await expect(templateLiteral).toHaveText(`${id.templateLiteral}: success`) // Template literal inline scripts loaded
await expect(dangerouslySet).toHaveText(`${id.dangerouslySet}: success`) // Dangerously set inline scripts loaded
await expect(scriptWithSrc).toHaveAttribute(`type`, `text/partytown-x`) // Scripts with sources loaded
})

// This behavior is broken upstream in Partytown, see https://github.com/BuilderIO/partytown/issues/74
test.skip(`should load successfully when navigating via Gatsby Link to a page with off-main-thread scripts`, async ({
page,
}) => {
await page.goto(`/`)
await page.locator(`[data-testid=off-main-thread]`).click()

const templateLiteral = await page
.locator(`[id=${id.templateLiteral}]`)
.textContent()
const partytownSnippet = page.locator(`[data-partytown]`)
const scriptWithSrc = page.locator(`[id=${id.three}]`)
const templateLiteral = page.locator(`[id=${id.templateLiteral}]`)
const dangerouslySet = page.locator(`[id=${id.dangerouslySet}]`)

await expect(partytownSnippet).toContainText(/THREE/) // Forwards configured
await expect(templateLiteral).toHaveText(`${id.templateLiteral}: success`) // Template literal inline scripts loaded
await expect(dangerouslySet).toHaveText(`${id.dangerouslySet}: success`) // Dangerously set inline scripts loaded
await expect(scriptWithSrc).toHaveAttribute(`type`, `text/partytown-x`) // Scripts with sources loaded, use `type` attr Partytown mutates on success as proxy
})

// This behavior is broken upstream in Partytown, see https://github.com/BuilderIO/partytown/issues/74
test.skip(`should load successfully when navigating via Gatsby Link between pages with off-main-thread scripts`, async ({
page,
}) => {
await page.goto(`/gatsby-script-off-main-thread/`)
await page.locator(`[data-testid=off-main-thread-2]`).click()

const dangerouslySet = await page
.locator(`[id=${id.dangerouslySet}]`)
.textContent()
const partytownSnippet = page.locator(`[data-partytown]`)
const templateLiteral = page.locator(`[id=${id.templateLiteral}-2]`)
const dangerouslySet = page.locator(`[id=${id.dangerouslySet}-2]`)
const scriptWithSrc = page.locator(`[id=${id.scriptWithSrc}]`)

await expect(scriptWithSrc).toBeTruthy()
await expect(templateLiteral).toEqual(`${id.templateLiteral}: success`)
await expect(dangerouslySet).toEqual(`${id.dangerouslySet}: success`)
await expect(partytownSnippet).toContainText(/some-other-forward/) // Forwards configured
await expect(templateLiteral).toHaveText(`${id.templateLiteral}-2: success`) // Template literal inline scripts loaded
await expect(dangerouslySet).toHaveText(`${id.dangerouslySet}-2: success`) // Dangerously set inline scripts loaded
await expect(scriptWithSrc).toHaveText(`${id.scriptWithSrc}: success`) // Scripts with sources loaded
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import * as React from "react"
import { Script } from "gatsby"

function OffMainThreadScriptsTwo() {
return (
<main style={{ margin: `1em` }}>
<h1>Script component e2e test</h1>

<Script
src="http://localhost:8000/used-by-off-main-thread-2.js"
strategy="off-main-thread"
forward={[`some-other-forward`]}
/>

<Script id="inline-script-template-literal" strategy="off-main-thread">
{createScript(`inline-script-template-literal-2`)}
</Script>

<Script
id="inline-script-dangerously-set"
strategy="off-main-thread"
dangerouslySetInnerHTML={{
__html: createScript(`inline-script-dangerously-set-2`),
}}
/>
</main>
)
}

function createScript(id) {
return `
{
const main = document.querySelector('main');
const elem = document.createElement('div');
elem.textContent = 'mutation-${id}: success';
elem.id = 'mutation-${id}';
main.appendChild(elem);
}
`
}

export default OffMainThreadScriptsTwo
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
import * as React from "react"
import { Script } from "gatsby"
import { Link, Script } from "gatsby"
import { scripts } from "../../gatsby-script-scripts"

function OffMainThreadScripts() {
return (
<main style={{ margin: `1em` }}>
<h1>Script component e2e test</h1>

<br />
<h2>Scripts with sources</h2>
<Link
to="/gatsby-script-off-main-thread-2/"
data-testid="off-main-thread-2"
>
Go to off-main-thread-2 scripts page
</Link>

<Script
id="three"
src={scripts.three}
strategy="off-main-thread"
forward={[`THREE`]}
Expand All @@ -33,11 +38,13 @@ function OffMainThreadScripts() {

function createScript(id) {
return `
const main = document.querySelector('main');
const elem = document.createElement('div');
elem.id = '${id}-mutation';
elem.textContent = '${id}-mutation: success';
main.appendChild(elem);
{
const main = document.querySelector('main');
const elem = document.createElement('div');
elem.textContent = 'mutation-${id}: success';
elem.id = 'mutation-${id}';
main.appendChild(elem);
}
`
}

Expand Down
3 changes: 3 additions & 0 deletions e2e-tests/development-runtime/src/pages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ const IndexPage = ({ data }) => (
</li>
))}
</ul>
<Link to="/gatsby-script-off-main-thread/" data-testid="off-main-thread">
Go to off-main-thread scripts page
</Link>
</Layout>
)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
const id = `script-with-src`
const main = document.querySelector(`main`)
const elem = document.createElement(`div`)
elem.textContent = `mutation-${id}: success`
elem.id = `mutation-${id}`
main.appendChild(elem)
}
5 changes: 4 additions & 1 deletion e2e-tests/production-runtime/gatsby-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,8 @@ module.exports = {
`gatsby-plugin-less`,
`gatsby-plugin-stylus`,
].concat(process.env.TEST_PLUGIN_OFFLINE ? [`gatsby-plugin-offline`] : []),
partytownProxiedURLs: [`https://unpkg.com/three@0.139.1/build/three.js`],
partytownProxiedURLs: [
`https://unpkg.com/three@0.139.1/build/three.js`,
`http://localhost:9000/used-by-off-main-thread-2.js`,
],
}
5 changes: 5 additions & 0 deletions e2e-tests/production-runtime/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ const config: PlaywrightTestConfig = {
reporter: "html",
/* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
use: {
// Use to slow down for debugging
// launchOptions: {
// slowMo: 5 * 1000,
// },
headless: !!process.env.CI,
/* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */
actionTimeout: 0,
/* Base URL to use in actions like `await page.goto('/')`. */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,65 @@
import { test, expect } from "@playwright/test"

const id = {
templateLiteral: `inline-script-template-literal-mutation`,
dangerouslySet: `inline-script-dangerously-set-mutation`,
// Acutual <Script>
three: `three`,

// DOM nodes appended as a result of `<Script>` execution
templateLiteral: `mutation-inline-script-template-literal`,
dangerouslySet: `mutation-inline-script-dangerously-set`,
scriptWithSrc: `mutation-script-with-src`,
}

test.describe(`off-main-thread scripts`, () => {
test(`should load successfully`, async ({ page }) => {
test(`should load successfully when the page is visited directly`, async ({
page,
}) => {
await page.goto(`/gatsby-script-off-main-thread/`)

// @ts-ignore
const scriptWithSrc = await page.evaluate(() => typeof THREE === `function`)
const partytownSnippet = page.locator(`[data-partytown]`)
const scriptWithSrc = page.locator(`[id=${id.three}]`)
const templateLiteral = page.locator(`[id=${id.templateLiteral}]`)
const dangerouslySet = page.locator(`[id=${id.dangerouslySet}]`)

await expect(partytownSnippet).toContainText(/THREE/) // Forwards configured
await expect(templateLiteral).toHaveText(`${id.templateLiteral}: success`) // Template literal inline scripts loaded
await expect(dangerouslySet).toHaveText(`${id.dangerouslySet}: success`) // Dangerously set inline scripts loaded
await expect(scriptWithSrc).toHaveAttribute(`type`, `text/partytown-x`) // Scripts with sources loaded
})

// This behavior is broken upstream in Partytown, see https://github.com/BuilderIO/partytown/issues/74
test.skip(`should load successfully when navigating via Gatsby Link to a page with off-main-thread scripts`, async ({
page,
}) => {
await page.goto(`/`)
await page.locator(`[data-testid=off-main-thread]`).click()

const templateLiteral = await page
.locator(`[id=${id.templateLiteral}]`)
.textContent()
const partytownSnippet = page.locator(`[data-partytown]`)
const scriptWithSrc = page.locator(`[id=${id.three}]`)
const templateLiteral = page.locator(`[id=${id.templateLiteral}]`)
const dangerouslySet = page.locator(`[id=${id.dangerouslySet}]`)

await expect(partytownSnippet).toContainText(/THREE/) // Forwards configured
await expect(templateLiteral).toHaveText(`${id.templateLiteral}: success`) // Template literal inline scripts loaded
await expect(dangerouslySet).toHaveText(`${id.dangerouslySet}: success`) // Dangerously set inline scripts loaded
await expect(scriptWithSrc).toHaveAttribute(`type`, `text/partytown-x`) // Scripts with sources loaded, use `type` attr Partytown mutates on success as proxy
})

// This behavior is broken upstream in Partytown, see https://github.com/BuilderIO/partytown/issues/74
test.skip(`should load successfully when navigating via Gatsby Link between pages with off-main-thread scripts`, async ({
page,
}) => {
await page.goto(`/gatsby-script-off-main-thread/`)
await page.locator(`[data-testid=off-main-thread-2]`).click()

const dangerouslySet = await page
.locator(`[id=${id.dangerouslySet}]`)
.textContent()
const partytownSnippet = page.locator(`[data-partytown]`)
const templateLiteral = page.locator(`[id=${id.templateLiteral}-2]`)
const dangerouslySet = page.locator(`[id=${id.dangerouslySet}-2]`)
const scriptWithSrc = page.locator(`[id=${id.scriptWithSrc}]`)

await expect(scriptWithSrc).toBeTruthy()
await expect(templateLiteral).toEqual(`${id.templateLiteral}: success`)
await expect(dangerouslySet).toEqual(`${id.dangerouslySet}: success`)
await expect(partytownSnippet).toContainText(/some-other-forward/) // Forwards configured
await expect(templateLiteral).toHaveText(`${id.templateLiteral}-2: success`) // Template literal inline scripts loaded
await expect(dangerouslySet).toHaveText(`${id.dangerouslySet}-2: success`) // Dangerously set inline scripts loaded
await expect(scriptWithSrc).toHaveText(`${id.scriptWithSrc}: success`) // Scripts with sources loaded
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import * as React from "react"
import { Script } from "gatsby"

function OffMainThreadScriptsTwo() {
return (
<main style={{ margin: `1em` }}>
<h1>Script component e2e test</h1>

<Script
src="http://localhost:9000/used-by-off-main-thread-2.js"
strategy="off-main-thread"
forward={[`some-other-forward`]}
/>

<Script id="inline-script-template-literal" strategy="off-main-thread">
{createScript(`inline-script-template-literal-2`)}
</Script>

<Script
id="inline-script-dangerously-set"
strategy="off-main-thread"
dangerouslySetInnerHTML={{
__html: createScript(`inline-script-dangerously-set-2`),
}}
/>
</main>
)
}

function createScript(id) {
return `
{
const main = document.querySelector('main');
const elem = document.createElement('div');
elem.id = 'mutation-${id}';
elem.textContent = 'mutation-${id}: success';
main.appendChild(elem);
}
`
}

export default OffMainThreadScriptsTwo
Loading

0 comments on commit 49cf094

Please sign in to comment.