Skip to content
Draft
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
5 changes: 5 additions & 0 deletions .changeset/weak-hovered-links.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: avoid retaining detached links and their subtrees during preloading
30 changes: 18 additions & 12 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -2327,31 +2327,35 @@ if (import.meta.hot) {

/**
* The anchor element whose href is being preloaded. It is reset after navigation
* or changes when a different anchor element is being preloaded.
* @type {{ element: Element | SVGAElement | undefined; href: string | SVGAnimatedString | undefined }}
* or changes when a different anchor element is being preloaded. Held weakly so
* removing a preloaded link does not retain its detached subtree.
* @type {{ element: WeakRef<HTMLAnchorElement | SVGAElement> | undefined; href: string | undefined }}
*/
let current_a = { element: undefined, href: undefined };

function setup_preload() {
/** @type {NodeJS.Timeout} */
let mousemove_timeout;
/** @type {HTMLAnchorElement | SVGAElement | undefined} */
// Removed links do not fire mouseleave, so do not keep them alive.
/** @type {WeakRef<HTMLAnchorElement | SVGAElement> | undefined} */
let hovered_a;
/** @type {PreloadDataPriority} */
let current_priority;

function clear_hover_preload() {
clearTimeout(mousemove_timeout);
hovered_a?.removeEventListener('mousemove', start_hover_preload);
hovered_a?.removeEventListener('mouseleave', clear_hover_preload);
const a = hovered_a?.deref();
a?.removeEventListener('mousemove', start_hover_preload);
a?.removeEventListener('mouseleave', clear_hover_preload);
hovered_a = undefined;
}

function start_hover_preload() {
clearTimeout(mousemove_timeout);
mousemove_timeout = setTimeout(() => {
if (!hovered_a) return;
void preload(hovered_a, PRELOAD_PRIORITIES.hover);
const a = hovered_a?.deref();
if (!a) return;
void preload(a, PRELOAD_PRIORITIES.hover);
}, 20);
}

Expand All @@ -2360,10 +2364,10 @@ function setup_preload() {
if (!(event.target instanceof Element)) return;

const a = find_anchor(event.target, container);
if (!a || a === hovered_a) return;
if (!a || a === hovered_a?.deref()) return;

clear_hover_preload();
hovered_a = a;
hovered_a = new WeakRef(a);

const options = get_router_options(a);

Expand Down Expand Up @@ -2414,9 +2418,11 @@ function setup_preload() {
* @param {PreloadDataPriority} priority
*/
async function preload(a, priority) {
// Store the value, not SVGAnimatedString, which also retains its owner.
const href = a instanceof SVGAElement ? a.href.baseVal : a.href;
// we don't want to preload data again if the user has already hovered/tapped
const interacted =
a === current_a.element && a?.href === current_a.href && priority >= current_priority;
a === current_a.element?.deref() && href === current_a.href && priority >= current_priority;
if (!a || interacted) return;

const { url, external, download } = get_link_info(a, base, app.hash);
Expand All @@ -2429,7 +2435,7 @@ function setup_preload() {
if (options.reload || same_url) return;

if (priority <= options.preload_data) {
current_a = { element: a, href: a.href };
current_a = { element: new WeakRef(a), href };
// we don't want to preload data again on tap if we've already preloaded it on hover
current_priority = PRELOAD_PRIORITIES.tap;

Expand All @@ -2444,7 +2450,7 @@ function setup_preload() {
void _preload_data(intent).catch(noop);
}
} else if (priority <= options.preload_code) {
current_a = { element: a, href: a.href };
current_a = { element: new WeakRef(a), href };
current_priority = priority;
void _preload_code(/** @type {URL} */ (url));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<script>
import { page } from '$app/state';

let open = $state(true);
const preload = page.url.searchParams.get('preload') ?? 'data';
</script>

<svelte:window onkeydown={(event) => event.key === 'Escape' && (open = false)} />

<main>
<a href="/data-sveltekit/preload-data/target" data-sveltekit-preload-data="hover">Navigate away</a
>

{#if open}
<div
role="dialog"
aria-label="Removable dialog"
data-sveltekit-preload-code={preload === 'code' ? 'hover' : 'false'}
data-sveltekit-preload-data={preload === 'data' ? 'hover' : 'false'}
>
{#if page.url.searchParams.get('svg') === 'true'}
<svg width="200" height="50">
<a href="/data-sveltekit/preload-data/target"><text x="0" y="25">Link in dialog</text></a>
</svg>
{:else}
<a href="/data-sveltekit/preload-data/target">Link in dialog</a>
{/if}
</div>
{/if}
</main>
95 changes: 95 additions & 0 deletions packages/kit/test/apps/basics/test/playwright/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,101 @@ test.describe('Invalidation', () => {
});

test.describe('data-sveltekit attributes', () => {
for (const preload of ['false', 'code', 'data']) {
for (const svg of [false, true]) {
test(`removed hovered ${svg ? 'SVG' : 'HTML'} links can be collected (preload: ${preload})`, async ({
page,
context,
browserName
}) => {
test.skip(browserName !== 'chromium', 'Requires CDP to force garbage collection');

await page.goto(`/data-sveltekit/preload-retention?preload=${preload}&svg=${svg}`);
const ref = await page.evaluateHandle(
() => new WeakRef(/** @type {Element} */ (document.querySelector('[role="dialog"]')))
);
await page.locator('[role="dialog"] a').hover();
// Let the rest timer and any preloading finish before removing the link.
await page.waitForTimeout(100);
await page.waitForLoadState('networkidle');
await page.keyboard.press('Escape');
await expect(page.getByRole('dialog')).toHaveCount(0);

const cdp = await context.newCDPSession(page);
await expect
.poll(async () => {
await cdp.send('HeapProfiler.collectGarbage');
return ref.evaluate((ref) => ref.deref() === undefined);
})
.toBe(true);
await ref.dispose();
await cdp.detach();
});
}
}

test('a page left by clicking a hovered link can be collected', async ({
page,
context,
browserName,
clicknav
}) => {
test.skip(browserName !== 'chromium', 'Requires CDP to force garbage collection');

await page.goto('/data-sveltekit/preload-retention');
const ref = await page.evaluateHandle(
() => new WeakRef(/** @type {Element} */ (document.querySelector('main')))
);
await clicknav('main > a');
await expect(page.locator('main')).toHaveCount(0);

const cdp = await context.newCDPSession(page);
await expect
.poll(async () => {
await cdp.send('HeapProfiler.collectGarbage');
return ref.evaluate((ref) => ref.deref() === undefined);
})
.toBe(true);
await ref.dispose();
await cdp.detach();
});

test('SVG links preload after href changes and code-to-data priority escalation', async ({
page
}) => {
/** @type {string[]} */
const requests = [];
page.on('request', (request) => {
if (request.url().includes('/preload-data/target/__data.json')) {
requests.push(request.url());
}
});

await page.goto('/data-sveltekit/preload-retention?preload=code&svg=true');
const link = page.locator('[role="dialog"] a');
await link.evaluate((a) => a.setAttribute('data-sveltekit-preload-data', 'tap'));
await link.hover();
await page.waitForTimeout(100);
await page.waitForLoadState('networkidle');
expect(requests).toHaveLength(0);

await link.dispatchEvent('touchstart');
await expect.poll(() => requests.length).toBe(1);
await page.waitForLoadState('networkidle');

// The first data preload clears current_a when it discards the old load cache.
// Tap again to exercise deduplication with an existing cache entry.
await link.dispatchEvent('touchstart');
await page.waitForTimeout(100);
await page.waitForLoadState('networkidle');
expect(requests).toHaveLength(1);

await link.evaluate((a) => a.setAttribute('href', '/data-sveltekit/preload-data/target?x=1'));
await link.dispatchEvent('touchstart');
await expect.poll(() => requests.length).toBe(2);
expect(new URL(requests[1]).searchParams.get('x')).toBe('1');
});

test('data-sveltekit-preload-code', async ({ page }) => {
/** @type {string[]} */
const responses = [];
Expand Down
Loading