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

Script and useScript #2194

Open
wants to merge 27 commits into
base: v1.x-2022-07
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8a7f93c
initial Script scaffold
juanpprieto Aug 12, 2022
0ea6916
Add script examples refactor strategies
juanpprieto Aug 16, 2022
ece42cb
improve types, refactor Script
juanpprieto Aug 16, 2022
ba75029
Improve types and refactor Script
juanpprieto Aug 16, 2022
ab5b61b
migrate from `strategy` to `load`, tests and refactoring
juanpprieto Sep 15, 2022
3654c6a
Add Script and useScript eslint rules
juanpprieto Sep 16, 2022
4c76136
refactor, cleanup and test Script(s) and loadScript(s)
juanpprieto Sep 16, 2022
6f0e64b
add useScript
juanpprieto Sep 16, 2022
a385d99
update experimental export
juanpprieto Sep 16, 2022
fce62a4
Add Script and useScript playground routes for e2e tests
juanpprieto Sep 16, 2022
645ba78
add Script and useScript e2e tests
juanpprieto Sep 16, 2022
5f8258d
add Script and useScript docs
juanpprieto Sep 26, 2022
9528a01
remove experimental
juanpprieto Sep 26, 2022
8aed11d
move Script to components
juanpprieto Sep 27, 2022
d034a82
update script docs
juanpprieto Sep 27, 2022
538ad4d
ignore Script lint rules in test routes
juanpprieto Sep 27, 2022
624cb4d
fix Script error in App.server.tsx, update partytown snippet
juanpprieto Sep 27, 2022
282b3f7
temporarily export Script from index
juanpprieto Sep 27, 2022
5feac32
Merge branch 'v1.x-2022-07' into @juanpprieto/Script
juanpprieto Sep 27, 2022
f742bad
re-enable all e2e tests
juanpprieto Sep 27, 2022
062b3b7
fix lint warning
juanpprieto Sep 27, 2022
f05677f
"fix" linting for CHANGELOG
juanpprieto Sep 28, 2022
0685b42
fix lint warnings
juanpprieto Sep 28, 2022
afe07f6
move Script e2e test to async-config
juanpprieto Sep 28, 2022
dc2ade1
remove tmp Script export
juanpprieto Sep 28, 2022
1d02f69
fix lint
juanpprieto Sep 28, 2022
56341a1
cleanup unnecessary async/await
juanpprieto Sep 28, 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
376 changes: 376 additions & 0 deletions docs/components/primitive/script.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,376 @@
---
gid: 5848138c-3dbb-11ed-b878-0242ac120002
title: Script
description: The Script component renders a script tag
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: The Script component renders a script tag
description: The Script component renders a script tag.

---

<aside class="note beta">
<h4>Experimental feature</h4>

<p>Hydrogen Script is an experimental feature. As a result, functionality is subject to change. You can provide feedback on this feature by <a href="https://github.com/Shopify/hydrogen/issues">submitting an issue in GitHub</a>.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<p>Hydrogen Script is an experimental feature. As a result, functionality is subject to change. You can provide feedback on this feature by <a href="https://github.com/Shopify/hydrogen/issues">submitting an issue in GitHub</a>.</p>
<p>The `Script` component is an experimental feature. As a result, functionality is subject to change. You can provide feedback on this feature by <a href="https://github.com/Shopify/hydrogen/issues">submitting an issue in GitHub</a>.</p>


</aside>

The `Script` component is an enhanced HTML [`<script>`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script) element. Script enables efficient loading third-party scripts by offering different loading strategies within the application lifecycle.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The `Script` component is an enhanced HTML [`<script>`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script) element. Script enables efficient loading third-party scripts by offering different loading strategies within the application lifecycle.
The `Script` component is an enhanced HTML [`<script>`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script) element. It offers different loading strategies within the application lifecycle. Use `Script` to load third-party scripts efficiently.

Maybe a use case here, like a for example, to help orient partners on when to use.


## Example code

Load google analytics only after react has finished hydrating on the client. "afterHydration" is the default loading strategy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Load google analytics only after react has finished hydrating on the client. "afterHydration" is the default loading strategy
Load Google Analytics only after React has finished hydrating on the client. 'afterHydration' is the default loading strategy.

Throughout, review for correct capitalization on Google


{% codeblock file, filename: 'App.server.jsx' %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{% codeblock file, filename: 'App.server.jsx' %}
{% codeblock file, filename: 'App.server.jsx' %}

hypo-nit. Just a linting thing on the shopify-dev repo. Would apply throughout

```tsx
import {Script} from '@shopify/hydrogen';

export default function App() {
return (
<>
//...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//...
...

I don't think that you need to comment out code elisions but I could be wrong.

<Script src="https://www.google-analytics.com/analytics.js" />
</>
)
}
```
{% endcodeblock %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{% endcodeblock %}
{% endcodeblock %}

apply throughout to minimize linting complains in shopify-dev?


## Props

You can customize the behavior of the component with the following props
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
You can customize the behavior of the component with the following props
You can customize the behavior of the component with the following props:


| Name | Type | Description |
| -------------- | ------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `target` | <code>"head" \| "body" (default)</code> | The target DOM element where the script should be inserted. This feature is only available to non-inline loading strategies such as "afterHydration", "inWorker" and "onIdle" |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `target` | <code>"head" \| "body" (default)</code> | The target DOM element where the script should be inserted. This feature is only available to non-inline loading strategies such as "afterHydration", "inWorker" and "onIdle" |
| `target` | <code>"head" \| "body" (default)</code> | The target DOM element where the script should be inserted. This feature is only available to non-inline loading strategies such as [`afterHydration`](LINK), [`inWorker`](LINK, and [`onIdle`](LINK). |

Apply links where required throughout, and apply code formatting over names where required, throughout.

| `id` | <code>string (required)</code> | A unique identifier for the script. The id will be used as the key of the script. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If id is required, shouldn't it be in the above example?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `id` | <code>string (required)</code> | A unique identifier for the script. The id will be used as the key of the script. |
| `id` | <code>string (required)</code> | A unique identifier for the script. The ID used as the key of the script. |

Unless representing as a literal value, upcase on ID.

| `src` | <code>string</code> | A URL string. This string can be an absolute path or a relative path depending on the location of the third-party script. The `src` prop is required if `dangerouslySetInnerHTML` or `children` are not used |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `src` | <code>string</code> | A URL string. This string can be an absolute path or a relative path depending on the location of the third-party script. The `src` prop is required if `dangerouslySetInnerHTML` or `children` are not used |
| `src` | <code>string</code> | A URL string. This string can be an absolute path or a relative path depending on the location of the third-party script. The `src` prop is required if `dangerouslySetInnerHTML` or `children` aren't used. |

Review the descriptions for period use
Verify throughout that you're using contractions

| `dangerouslySetInnerHTML` | <code>string</code> | Any valid javascript code |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `dangerouslySetInnerHTML` | <code>string</code> | Any valid javascript code |
| `dangerouslySetInnerHTML` | <code>string</code> | Any valid JavaScript code. |

and below

| `children` | <code>string \| string[]</code> | Any valid javascript code |
| `load` | <code>"beforeHydration" \| "afterHydration" (default) \| "inWorker" \| "onIdle"</code>| The loading strategy. See Loading strategies for more info. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `load` | <code>"beforeHydration" \| "afterHydration" (default) \| "inWorker" \| "onIdle"</code>| The loading strategy. See Loading strategies for more info. |
| `load` | <code>"beforeHydration" \| "afterHydration" (default) \| "inWorker" \| "onIdle"</code>| The loading strategy. Learn more about [loading strategies](#loading-strategies). |

do you need the quotes if the phrase is in code? beforeHydration etc.

Revise references to see, as not all users are sighted

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might want to bikeshed these strategy names

| `reload` | <code>boolean (default false)</code> | Scripts rendered with this option will be reloaded after every page navigation (if available on the next route). This option is only available in "afterHydration" and "onIdle" loading strategies. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `reload` | <code>boolean (default false)</code> | Scripts rendered with this option will be reloaded after every page navigation (if available on the next route). This option is only available in "afterHydration" and "onIdle" loading strategies. |
| `reload` | <code>boolean (default `false`)</code> | Scripts are reloaded after every page navigation if the next route uses [`afterHydration`](LINK) or [`onIdle`](LINK) loading strategies. |

Not sure if I got this correctly. Just figured that we could resolve some ambiguity and tighten up the description a bit.

| `onLoad` | <code>(script) => void</code> | A callback that fires when a script is loaded. This callback is only available in "afterHydration" and "onIdle" loading strategies |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `onLoad` | <code>(script) => void</code> | A callback that fires when a script is loaded. This callback is only available in "afterHydration" and "onIdle" loading strategies |
| `onLoad` | <code>(script) => void</code> | A callback that fires when a script is loaded using [`afterHydration`](#1463 ) or [`onIdle`](#LINK) loading strategies. |

| `onReady` | <code>(script) => void</code> | A callback that fires when a script is successfully loaded and run. This callback is only available in "afterHydration" and "onIdle" loading strategies |
| `onError` | <code>(script) => void</code> | A callback that fires when a script fail to load. This callback is only available in "afterHydration" and "onIdle" loading strategies |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `onError` | <code>(script) => void</code> | A callback that fires when a script fail to load. This callback is only available in "afterHydration" and "onIdle" loading strategies |
| `onError` | <code>(script) => void</code> | A callback that fires when a script doesn't load using [`afterHydration`](#LINK) or [`onIdle`](#LINK) loading strategies. |



## Loading strategies

### `beforeHydration`

These scripts are inlined and hence considered render-blocking. This strategy is mainly recommended for scripts aiming to set global `window` properties, configurations or event listeners. These scripts can only be included in `App.server.tsx`. This ensures that they are run on any initial route.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
These scripts are inlined and hence considered render-blocking. This strategy is mainly recommended for scripts aiming to set global `window` properties, configurations or event listeners. These scripts can only be included in `App.server.tsx`. This ensures that they are run on any initial route.
Scripts using the `beforeHydration` strategy are inlined and render-blocking. This strategy is only recommended for scripts that set global `window` properties, configurations or event listeners. These scripts can only be included in `App.server.tsx`, which ensures they are run on any initial route.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#The that after ensures is correct, let's put it back in!

To @cartogram's edit, would add an Oxford comma between configurations and event listeners

Verify throughout that you're using the Oxford comma


{% codeblock file, filename: 'App.server.jsx' %}
```jsx
/* App.server.tsx */
import {Script} from '@shopify/hydrogen';

export default function App() {
return (
<>
// via dangerouslySetInnerHTML
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// via dangerouslySetInnerHTML
// Set using dangerouslySetInnerHTML

<Script
id="bh-dangerously"
load="beforeHydration"
dangerouslySetInnerHTML={{
__html: `
window.dataLayer = window.dataLayer || [];
`,
}}
/>

// or via children prop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// or via children prop
// Set using the children prop

<Script id="bh-children" load="beforeHydration">
{`window.dataLayer = window.dataLayer || [];`}
</Script>

// or via src — not recommended as this would block the main thread.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// or via src — not recommended as this would block the main thread.
// Set using src. Not recommended, because this would block the main thread.

// use afterHydration or onIdle instead to improve performance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// use afterHydration or onIdle instead to improve performance
// To improve performance, use afterHydration or onIdle loading strategies instead.

<Script
id="bh-src"
load="beforeHydration"
src="//example.com/script.js"
/>
</>
)
}
```
{% endcodeblock %}

### `afterHydration`

These scripts are loaded, injected and run after react has finished hydrating on the client (via a useEffect). In addition, these scripts include additional props to further customize their behavior `target`, `reload`, `onLoad`, `onReady` and `onError` callbacks.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
These scripts are loaded, injected and run after react has finished hydrating on the client (via a useEffect). In addition, these scripts include additional props to further customize their behavior `target`, `reload`, `onLoad`, `onReady` and `onError` callbacks.
Scripts using the `afterHydration` strategy are loaded, injected and run after React has finished hydrating on the client (via a `useEffect`). In addition, these scripts accept props to further customize the behavior of the `target`, `reload`, `onLoad`, `onReady` and `onError` callbacks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The beforeHydration strategy gives an example how or why it would be used. Maybe provide the same for afterHydration.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you link to the table(s) that discuss reload, onLoad, etc?


{% codeblock file, filename: 'Component.client.jsx' %}
```jsx
import {Script} from '@shopify/hydrogen';

function Component() {
return (
<>
// with children
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// with children
// with children and reload

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caps to start comments. So With here. Apply throughout.

<Script
id="oi-children-reload"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
id="oi-children-reload"
id="ah-children-reload"

load="afterHydration"
reload={true}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
reload={true}
reload

Make this change for all of these I think.

>
{`console.log('Loaded after hydration!. Will re-execute if navigating to another route that also renders <Component />.`}
</Script>

// with a src
<Script
id="oi-src"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
id="oi-src"
id="ah-src"

load="afterHydration"
src="//example.com/script.js"
/>

// with a src and with reload
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// with a src and with reload
// with a src and reload

<Script
id="oi-src-reload"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
id="oi-src-reload"
id="ah-src-reload"

src="//example.com/script.js"
load="afterHydration"
reload={true}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
reload={true}
reload

/>

// with callbacks
<Script
id="oi-src-cbs"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
id="oi-src-cbs"
id="ah-src-cbs"

src="//example.com/script.js"
load="afterHydration"
onLoad={() => {
console.log('🌕 onLoad event');
}}
onReady={() => {
console.log('🟢 onReady event');
}}
onError={(error) => {
console.error('🔴 onError event', error);
}}
/>
</>
)
}
```
{% endcodeblock %}

### `onIdle`

These scripts are loaded, injected and run when the main thread is idle (via requestIdleCallback). In addition, these scripts include additional props to further customize their behavior `target`, `reload`, `onLoad`, `onReady` and `onError` callbacks.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you end up making the suggestions above, please apply them to this paragraph as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
These scripts are loaded, injected and run when the main thread is idle (via requestIdleCallback). In addition, these scripts include additional props to further customize their behavior `target`, `reload`, `onLoad`, `onReady` and `onError` callbacks.
These scripts are loaded, injected and run when the main thread is idle (via `requestIdleCallback`). In addition, these scripts include additional props to further customize their behavior, including`target`, `reload`, `onLoad`, `onReady`, and `onError` callbacks.


{% codeblock file, filename: 'Component.client.jsx' %}
```jsx
import {Script} from '@shopify/hydrogen';

function Component() {
return (
<>
// with children
<Script
id="oi-children-reload"
load="onIdle"
reload={true}
>
{`console.log('Loaded when on idle!. Will re-execute if navigating to another route that also renders <Component />.`}
</Script>

// with a src
<Script
id="oi-src"
load="onIdle"
src="//example.com/script.js"
/>

// with a src and with reload
<Script
id="oi-src-reload"
src="//example.com/script.js"
load="onIdle"
reload={true}
/>

// with callbacks
<Script
id="oi-src-cbs"
src="//example.com/script.js"
load="onIdle"
onLoad={() => {
console.log('🌕 onLoad event');
}}
onReady={() => {
console.log('🟢 onReady event');
}}
onError={(error) => {
console.error('🔴 onError event', error);
}}
/>
</>
)
}
```
{% endcodeblock %}

### `inWorker`

These scripts are run outside the main thread by leveraging a [Partytown](https://partytown.builder.io/) web worker. To enable support for this strategy follow these steps:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scripts using the inWorker etc. Basically @cartogram's edits to the other descriptions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
These scripts are run outside the main thread by leveraging a [Partytown](https://partytown.builder.io/) web worker. To enable support for this strategy follow these steps:
These scripts are run outside the main thread by leveraging a [Partytown](https://partytown.builder.io/) web worker. Enable support for this strategy with the following steps:


##### 1. Install partytown
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
##### 1. Install partytown
##### Step 1. Install partytown

Add the Step N to the rest


```terminal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```terminal
```bash

yarn add @builder.io/partytown
```

##### 2. Add partytown copylib script
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
##### 2. Add partytown copylib script
##### Step 2. Add the partytown copylib script


{% codeblock file, filename: 'package.json' %}
```
{
...
"scripts": {
"dev": "npm run partytown && shopify hydrogen dev",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it matter that yarn is used above but this object has npm values?

"build": "npm run partytown && shopify hydrogen build",
"partytown": "partytown copylib public/~partytown",
...
}
}
```
{% endcodeblock %}

##### 3. Import `Partytown` and enable atomic mode
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some differences in capitalization and code formatting around Partytown, just wanted to make sure that was intentional


{% codeblock file, filename: 'App.server.tsx' %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On line 59 you've got App.server.jsx, with the .tsx filename in comments. Would something similar be done here?

```jsx
import {Script} from '@shopify/hydrogen';
import {partytownSnippet} from '@builder.io/partytown/integration';

/*
Set the required response headers to enable partytown atomics
@see: https://partytown.builder.io/atomics
*/
function enablePartytownAtomic(response) {
response.headers.set('Cross-Origin-Embedder-Policy', 'credentialless');
response.headers.set('Cross-Origin-Opener-Policy', 'same-origin');
}

export default function App({request, response}) {
enablePartytownAtomic(response);

return (
<>
// Initialize and configure partytown
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering why the mix of comment styles, e.g. /* foo */ and // foo

<Script id="partytown-snippet" load="onIdle">
{partytownSnippet({
forward: ['forwardedTestFn'],
resolveUrl(url, location, type) {
// Some 3rd party libs/resources like https://www.googletagmanager.com/gtm.js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Some 3rd party libs/resources like https://www.googletagmanager.com/gtm.js
// Some 3rd party libraries and resources like https://www.googletagmanager.com/gtm.js

// require a reverse proxy to handle CORS via when loaded via Web Worker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is via when/via when loaded via intentional?

const isScriptReq = type === 'script';
const isProxyReq = url.href.includes('/reverse-proxy');
const isCorsReq =
url.href.includes('cors=true') || url.href.includes('gtm.js');

if (isScriptReq && isCorsReq && !isProxyReq) {
const proxyUrl = new URL(location.origin + '/reverse-proxy');
proxyUrl.searchParams.append('libUrl', url.href);
return proxyUrl;
}
return url;
},
})}
</Script>
<ShopifyProvider countryCode={countryCode}>
....
</>
)
}
```
{% endcodeblock %}

#### 4. Add a reverse proxy (optional)

The following reverse proxy can be integrated to provide the correct CORS headers for libraries that require them when loaded within a web worker. For more information please check [proxying-requests](https://partytown.builder.io/proxying-requests)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The following reverse proxy can be integrated to provide the correct CORS headers for libraries that require them when loaded within a web worker. For more information please check [proxying-requests](https://partytown.builder.io/proxying-requests)
Integrate the following reverse proxy to provide the correct CORS headers for libraries that require them when loaded within a web worker. For more information please check [proxying-requests](https://partytown.builder.io/proxying-requests)

above you use Web Worker (caps, no article), but here it's a web worker. Is there one pattern to rule them all? If so, review throughout and apply.


{% codeblock file, filename: '/src/routes/reverse-proxy.server.js' %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious re. .js file format, but /* foo */ commenting and .jsx syntax highlighting.

```jsx
/*
Reverse proxies partytown libs that require CORS. Used by Partytown resolveUrl
@see: https://partytown.builder.io/proxying-requests
@see: https://developers.cloudflare.com/workers/examples/cors-header-proxy/
*/

// The endpoint you want the CORS reverse proxy to be on
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// The endpoint you want the CORS reverse proxy to be on
// The endpoint that you want the CORS reverse proxy to be on.

Should be consistent with using periods at the end of full sentences and multi-sentence comments.

const PROXY_ENDPOINT = '/reverse-proxy';

export async function api(request) {
const url = new URL(request.url);
const isProxyReq = url.pathname.startsWith(PROXY_ENDPOINT);
const isGet = request.method === 'GET';

if (isProxyReq && isGet) {
// Handle requests to the API server
return handleRequest(request);
} else {
return new Response(null, {
status: 405,
statusText: 'Only proxy requests are allowed',
});
}
}

async function handleRequest(request) {
const url = new URL(request.url);
// The target lib url
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// The target lib url
// The target library URL

let libUrl = url.searchParams.get('libUrl');

if (libUrl == null) {
libUrl = request.url.href;
}

try {
let response = await fetch(libUrl);
const body = await response.arrayBuffer();
const contentType = response.headers.get('content-type');
const cacheControl = response.headers.get('cache-control');

// Recreate the response so you can modify the headers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Recreate the response so you can modify the headers
// Recreate the response so that you can modify the headers.

response = new Response(body, {
headers: {
'content-type': contentType,
'Access-Control-Allow-Origin': url.origin,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is an Access-Control-Allow_Origin header necessary? Just by going through the proxy we are already on the same domain right?

'cache-control': `${cacheControl}`,
Vary: 'Origin', // Append to/Add Vary header so browser will cache response correctly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Vary: 'Origin', // Append to/Add Vary header so browser will cache response correctly
Vary: 'Origin', // Append to or add the Vary header so that the browser caches responses correctly.

},
status: 200,
});

return response;
} catch (error) {
return new Response(error, {status: 500});
}
}
```
{% endcodeblock %}

#### 5. Add inWorker scripts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#### 5. Add inWorker scripts
#### 5. Add `inWorker` scripts

etc.

{% codeblock file, filename: 'App.server.tsx' %}
```jsx
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```jsx
```tsx

Would review file format and syntax highlighting combos in case there are other inconsistencies

export default function App() {
return (
<>
// Loading google analytics.js via a web worker
<Script
id="inWorker-analytics"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
id="inWorker-analytics"
id="iw-analytics"

I don't really care how these IDs are named as long as it is consistent.

load="inWorker"
src="https://www.google-analytics.com/analytics.js?cors=true"
/>

// Loading google gtm.js via a web worker
<Script
id="inWorker-gtm"
load="inWorker"
src="https://www.googletagmanager.com/gtm.js?id=GTM-XYZ12345"
/>
</>
)
}
```
{% endcodeblock %}

## Related components

- [`useScript`](https://shopify.dev/api/hydrogen/hooks/primitive/usescript)
Loading