Skip to content
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
4 changes: 4 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions test-app/app/templates/application.gts
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,10 @@
<a href="/does-not-exist">here</a>
<a href="#title">here</a>
<a href="/#title">/ here</a>

{{! non-SPA links: handled by the browser, not the router — visitAllLinks must skip them }}
<a href="/foo" target="_blank" rel="noopener noreferrer">new tab</a>
<a href="/some-report/index.html" download>download</a>
<a href="/foo" rel="external">external</a>
<a href="mailto:someone@example.com">email</a>
</template>
19 changes: 19 additions & 0 deletions test-app/tests/acceptance/visit-all-links-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,23 @@ module('All Links', function (hooks) {
`multiple usages does not visit different numbers of links (${size1} === ${size2})`,
);
});

test('non-SPA links are skipped', async function (assert) {
// The application template renders target="_blank", download,
// rel="external", and mailto: links. The browser handles those natively —
// clicking them can never change currentURL() — so a crawl that clicked
// them would report failed navigations. Every URL the crawl does visit
// must be one the router can serve.
const visited: string[] = [];

await visitAllLinks((url) => {
visited.push(url);

const isNonSPA = url.startsWith('mailto:') || url.endsWith('.html');

assert.false(isNonSPA, `${url} is a route the router handles`);
});

assert.ok(visited.length > 0, 'the SPA links were still visited');
});
});
3 changes: 2 additions & 1 deletion test-support/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@
},
"dependencies": {
"@ember/test-helpers": "^4.0.4 || ^5.2.2",
"@embroider/addon-shim": "^1.8.7"
"@embroider/addon-shim": "^1.8.7",
"should-handle-link": "^1.2.2"
},
"devDependencies": {
"@babel/core": "^7.23.6",
Expand Down
22 changes: 21 additions & 1 deletion test-support/src/routing/visit-all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
findAll,
} from '@ember/test-helpers';
import QUnit from 'qunit';
import { shouldHandle } from 'should-handle-link';
import type Owner from '@ember/owner';
import type RouterService from '@ember/routing/router-service';

Expand All @@ -23,10 +24,29 @@ function findInAppLinks(): InAppLink[] {
const allAnchorsOnThePage = findAll('a');

for (const a of allAnchorsOnThePage) {
// `findAll('a')` can also match SVG `<a>`, whose `href` is an
// SVGAnimatedString the router (and shouldHandle) can't work with.
if (!(a instanceof HTMLAnchorElement)) continue;

const href = a.getAttribute('href');

if (!href) continue;
if (href.startsWith('http')) continue;

/**
* Links the SPA's router never handles are handled natively by the
* browser instead (new tab/window via `target`, download dialog,
* `mailto:`/`tel:`, cross-origin, `rel="external"`). Clicking them in a
* test can't change `currentURL()`, so they'd always be reported as
* failed navigations.
*
* `shouldHandle` is the predicate ember-primitives' @properLinks uses to
* decide whether the router handles a click, so the crawler visits
* exactly the set of links the router would. The fabricated click event
* carries the "plain left click" defaults (button 0, no modifier keys).
*/
if (!shouldHandle(window.location.href, a, new MouseEvent('click'))) {
continue;
}

const current = new URL(currentURL(), window.location.origin);
const url = new URL(href, current);
Expand Down
Loading