From c01b891bc2ed15d0fad6354130e686c17896f5b9 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 28 Jul 2026 18:14:03 -0400 Subject: [PATCH] visitAllLinks: visit-mode crawls with one render per target (new default) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The click-based crawl does visit(returnTo) + click for every processed link: two page renders per unique target. In apps whose pages are expensive to render — e.g. docs apps that runtime-compile markdown and live demos — that constant is the whole wall-clock: a 265-page app measured ~4.8s per target, and the full crawl exceeded 30 minutes. 'visit' mode (now the default) navigates straight to each target and collects its links there: one render per unique URL, the floor for a full crawl. Reachability is identical (covered by a parity test), and what click mode uniquely exercised — link interception, the relative-href rewrite, non-SPA skips — is covered once by this repo's own test-app instead of once per link per consuming app. visit() also surfaces route errors directly, which the crawl reports as a failed navigation with the error text. 'click' mode remains available via visitAllLinks(cb, redirects, { mode: 'click' }) for apps that want per-link click fidelity. The relative-href authoring warning fires in both modes. Co-Authored-By: Claude Fable 5 --- .../tests/acceptance/visit-all-links-test.ts | 22 ++++++ test-support/src/routing/visit-all.ts | 75 ++++++++++++++----- 2 files changed, 78 insertions(+), 19 deletions(-) diff --git a/test-app/tests/acceptance/visit-all-links-test.ts b/test-app/tests/acceptance/visit-all-links-test.ts index b799a8b..f2961ff 100644 --- a/test-app/tests/acceptance/visit-all-links-test.ts +++ b/test-app/tests/acceptance/visit-all-links-test.ts @@ -73,6 +73,28 @@ module('All Links', function (hooks) { ); }); + test('click mode crawls the same targets as visit mode', async function (assert) { + // 'visit' (the default) navigates straight to each target — one render + // per unique URL. 'click' returns to the source page and clicks the real + // anchor (including the relative-href rewrite). Same reachability either + // way. + const viaVisit: string[] = []; + const viaClick: string[] = []; + + await visitAllLinks((url) => { + viaVisit.push(url); + }); + await visitAllLinks( + (url) => { + viaClick.push(url); + }, + undefined, + { mode: 'click' }, + ); + + assert.deepEqual(viaClick.sort(), viaVisit.sort(), 'both modes crawl the same URLs'); + }); + test('each target is visited once', async function (assert) { // `visited` is keyed on the target path alone (not (page, target) pairs): // shared links — like this app's application-template nav — appear on diff --git a/test-support/src/routing/visit-all.ts b/test-support/src/routing/visit-all.ts index 29ce70a..8aef06e 100644 --- a/test-support/src/routing/visit-all.ts +++ b/test-support/src/routing/visit-all.ts @@ -64,10 +64,27 @@ function findInAppLinks(): InAppLink[] { const assert = QUnit.assert; +interface VisitAllLinksOptions { + /** + * How each discovered target is navigated to: + * + * - `'visit'` (the default): `visit(target)` directly. One page render per + * unique target — the cheapest possible full crawl. + * - `'click'`: return to the page the link was found on and click the + * actual anchor, exercising the app's link-interception (e.g. + * `@properLinks`) for every link. Twice the page renders of `'visit'` + * (each processed link re-renders its source page), so reserve it for + * apps that need per-link click fidelity. + */ + mode?: 'visit' | 'click'; +} + export async function visitAllLinks( callback?: (url: string) => void | Promise, knownRedirects?: Record, + options?: VisitAllLinksOptions, ) { + const mode = options?.mode ?? 'visit'; /** * app-relative target paths (without hash) */ @@ -129,35 +146,55 @@ export async function visitAllLinks( continue; } - await visit(returnTo); - - const link = find(toVisit.selector); - - debugAssert(`link exists via selector \`${toVisit.selector}\``, link); - - /** - * The click navigates by `element.href`, which the browser resolved - * against the test page's URL (e.g. `/tests`) — NOT against the app's - * current route the way a production visit would (there, the address bar - * is the current route). A relative href would therefore navigate - * somewhere the real app never goes. We already resolved the target - * against `currentURL()` when the link was encountered, so point the - * anchor at that; the click then exercises the real - * properLinks-and-router path with the production URL. - */ if (!toVisit.original.startsWith('/')) { console.warn( `[visitAllLinks] Relative href "${toVisit.original}" found on ${returnTo}. ` + `Relative hrefs resolve against the browser's URL rather than the app's current route, ` + `so they only behave in a real full-page visit — they misresolve in this test harness ` + `and under any mount where the address bar isn't the route (embeds, previews). ` + - `The crawler pointed this click at the resolved target instead. ` + + `The crawler navigated to the resolved target instead. ` + `Action: update the source document to link to "${toVisit.href}" directly.`, ); - link.setAttribute('href', toVisit.href); } - await click(link); + if (mode === 'click') { + await visit(returnTo); + + const link = find(toVisit.selector); + + debugAssert(`link exists via selector \`${toVisit.selector}\``, link); + + /** + * The click navigates by `element.href`, which the browser resolved + * against the test page's URL (e.g. `/tests`) — NOT against the app's + * current route the way a production visit would (there, the address + * bar is the current route). A relative href would therefore navigate + * somewhere the real app never goes. We already resolved the target + * against `currentURL()` when the link was encountered, so point the + * anchor at that; the click then exercises the real + * properLinks-and-router path with the production URL. + */ + if (!toVisit.original.startsWith('/')) { + link.setAttribute('href', toVisit.href); + } + + await click(link); + } else { + try { + await visit(nonHashPart ?? toVisit.href); + } catch (error) { + // visit() rejects when the route errors (click-mode surfaces the + // same problem as a URL mismatch via the app's error substate) + assert.pushResult({ + result: false, + actual: String(error), + expected: nonHashPart, + message: `Navigation was successful: to:${toVisit.original}, from:${returnTo}`, + }); + visited.add(key); + continue; + } + } const current = rootURL.replace(/\/$/, '') + '/' + currentURL().replace(/^\//, '');