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.

Sign up for GitHub

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(perf): remove DOM mutation overhead from fog-of-war prefetch #12542

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(perf): remove DOM mutation overhead from fog-of-war prefetch #12542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a814e6e
fix(perf): remove DOM mutation overhead from fog-of-war prefetch
developit Dec 13, 2024
7be08f9
Add myself to the CLA
developit Dec 13, 2024
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 contributors.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
- david-crespo
- dcblair
- decadentsavant
- developit
- dgrijuela
- DigitalNaut
- dmitrytarassov
Expand Down
52 changes: 16 additions & 36 deletions packages/react-router/lib/dom/ssr/fog-of-war.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,23 @@ export function useFogOFWarDiscovery(
if (!path) {
return;
}
let url = new URL(path, window.location.origin);
if (!discoveredPaths.has(url.pathname)) {
nextPaths.add(url.pathname);
// optimization: use the already-parsed pathname from links
let pathname =
el.tagName === 'A'
? (el as HTMLAnchorElement).pathname
Copy link

Choose a reason for hiding this comment

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

It almost certainly doesn't matter, but <area> also has these helpers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this only runs on <a> and <form> (I assume because remix doesn't support other types of links?)

: new URL(path, window.location.origin).pathname;
if (!discoveredPaths.has(pathname)) {
nextPaths.add(pathname);
}
}

// Register and fetch patches for all initially-rendered links/forms
async function fetchPatches() {
// re-check/update registered links
document
.querySelectorAll("a[data-discover], form[data-discover]")
.forEach(registerElement);

let lazyPaths = Array.from(nextPaths.keys()).filter((path) => {
if (discoveredPaths.has(path)) {
nextPaths.delete(path);
Expand All @@ -150,43 +159,14 @@ export function useFogOFWarDiscovery(
}
}

// Register and fetch patches for all initially-rendered links
document.body
.querySelectorAll("a[data-discover], form[data-discover]")
.forEach((el) => registerElement(el));
let debouncedFetchPatches = debounce(fetchPatches, 100);

// scan and fetch initial links
fetchPatches();

// Setup a MutationObserver to fetch all subsequently rendered links/form
let debouncedFetchPatches = debounce(fetchPatches, 100);

function isElement(node: Node): node is Element {
return node.nodeType === Node.ELEMENT_NODE;
}

let observer = new MutationObserver((records) => {
let elements = new Set<Element>();
records.forEach((r) => {
[r.target, ...r.addedNodes].forEach((node) => {
if (!isElement(node)) return;
if (node.tagName === "A" && node.getAttribute("data-discover")) {
elements.add(node);
} else if (
node.tagName === "FORM" &&
node.getAttribute("data-discover")
) {
elements.add(node);
}
if (node.tagName !== "A") {
node
.querySelectorAll("a[data-discover], form[data-discover]")
.forEach((el) => elements.add(el));
}
});
});
elements.forEach((el) => registerElement(el));
debouncedFetchPatches();
});
// It just schedules a full scan since that's faster than checking subtrees
let observer = new MutationObserver(debouncedFetchPatches);

observer.observe(document.documentElement, {
subtree: true,
Expand Down
Loading