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(patchUrlMappings): recreate script elements when patching their src attribute #242

Merged
merged 2 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions docs/local-sdk-development.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ To test changes to the SDK, locally, with either of these projects, you must do

1. From terminal, navigate to the root of either project's directory, (i.e. examples/discord-application-start or examples/sdk-playground)
2. Start up the example's web server with `pnpm dev`.
3. In another terminal, start up the SDK's development pipeline with `pnpm dev` from the root of the repository.
30 changes: 28 additions & 2 deletions src/utils/patchUrlMappings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ export function patchUrlMappings(
if (mutation.type === 'attributes' && mutation.attributeName === 'src') {
attemptSetNodeSrc(mutation.target, mappings);
} else if (mutation.type === 'childList') {
mutation.addedNodes.forEach((node) => recursivelyRemapChildNodes(node, mappings));
mutation.addedNodes.forEach((node) => {
attemptSetNodeSrc(node, mappings);
recursivelyRemapChildNodes(node, mappings);
});
}
}
};
Expand Down Expand Up @@ -128,7 +131,30 @@ function attemptSetNodeSrc(node: Node, mappings: Mapping[]) {
if (node instanceof HTMLElement && node.hasAttribute('src')) {
const url = absoluteURL(node.getAttribute('src') ?? '');
if (url.host === window.location.host) return;
node.setAttribute('src', attemptRemap({url, mappings}).toString());

if (node.tagName.toLowerCase() === 'script') {
// Scripts are a special case, and need to be wholly recreated since
// modifying a script tag doesn't refetch.
attemptRecreateScriptNode(node, {url, mappings});
} else {
node.setAttribute('src', attemptRemap({url, mappings}).toString());
}
}
}

function attemptRecreateScriptNode(node: HTMLElement, {url, mappings}: RemapInput) {
const newUrl = attemptRemap({url, mappings});
if (url.toString() !== newUrl.toString()) {
// Note: Script tags cannot be duplicated via `node.clone()` because their internal 'already started'
// state prevents the new one from being fetched. We must manually recreate the duplicate tag instead.
const newNode = document.createElement(node.tagName);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't node.clone() here because script nodes have internal state on whether or not the node has started its fetch. See https://html.spec.whatwg.org/multipage/scripting.html and search the page for "already started", or this StackOverflow answer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks great. Thanks for calling out how we got here. It may be worth leaving a comment to explain the motivation

Well done 👏🏻

newNode.innerHTML = node.innerHTML;
for (const attr of node.attributes) {
newNode.setAttribute(attr.name, attr.value);
}
newNode.setAttribute('src', attemptRemap({url, mappings}).toString());
node.after(newNode);
node.remove();
}
}

Expand Down
1 change: 1 addition & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"extends": "./tsconfig-base.json",
"compilerOptions": {
"lib": ["DOM", "DOM.Iterable", "ESNext"],
"rootDir": "src",
// TODO: Enable this check. It is stricter.
// https://app.asana.com/0/1202090529698493/1205406173366739/f
Expand Down