Skip to content

fix(no-navigation-without-resolve): do not report if there is data-sveltekit-reload or rel="external" in <a>#1380

Open
baseballyama wants to merge 1 commit intomainfrom
fix/no-navigation-without-resolve
Open

fix(no-navigation-without-resolve): do not report if there is data-sveltekit-reload or rel="external" in <a>#1380
baseballyama wants to merge 1 commit intomainfrom
fix/no-navigation-without-resolve

Conversation

@baseballyama
Copy link
Member

@baseballyama baseballyama commented Sep 23, 2025

close part of #1353

There haven’t been any reports yet, but I realized we’re missing handling for another way of specifying external links.

@changeset-bot
Copy link

changeset-bot bot commented Sep 23, 2025

🦋 Changeset detected

Latest commit: 35c72fc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-plugin-svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

…veltekit-reload` or `rel="external"` in `<a>`
@baseballyama baseballyama force-pushed the fix/no-navigation-without-resolve branch from 7d946fc to 35c72fc Compare September 23, 2025 13:13
@github-actions
Copy link
Contributor

github-actions bot commented Sep 23, 2025

Try the Instant Preview in Online Playground

ESLint Online Playground

Install the Instant Preview to Your Local

npm i https://pkg.pr.new/eslint-plugin-svelte@35c72fc

Published Instant Preview Packages:

View Commit

@marekdedic
Copy link
Contributor

I understand rel="external" (and it was actually reported in some related issue), but not the data-sveltekit-reload - why should that one be treated as external?

) {
return;
}
if (anchorHasSveltekitReload(node) || anchorRelIncludesExternal(node)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add this to the previous if?

const relAttr = startTag.attributes.find((attr): attr is AST.SvelteAttribute => {
return attr.type === 'SvelteAttribute' && attr.key.name === 'rel';
});
if (!relAttr) return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!relAttr) return false;
if (relAttr === undefined) return false;

}

function relTokenListIncludesExternal(value: string): boolean {
return /(?:^|\s)external(?:\s|$)/i.test(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return /(?:^|\s)external(?:\s|$)/i.test(value);
return value.split(" ").some((part) => part === "external");

// Handle literal values like rel="external" or rel="external nofollow"
for (const v of relAttr.value) {
if (v.type === 'SvelteLiteral') {
if (relTokenListIncludesExternal(v.value)) return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Merge with parent if

// Best-effort: detect simple string literals in mustache, e.g., rel={'external'}
const expr = v.expression;
if (expr.type === 'Literal' && typeof expr.value === 'string') {
if (relTokenListIncludesExternal(expr.value)) return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Merge with parent if

expr.expressions.length === 0 &&
expr.quasis.length === 1
) {
if (relTokenListIncludesExternal(expr.quasis[0].value.raw)) return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Merge with parent if

@sacrosanctic
Copy link
Contributor

data-sveltekit-reload causes a hard refresh, but if the target is an internal link, it should still be linted.

@marekdedic
Copy link
Contributor

@baseballyama Most of this PR (the rel="external" part) has been solved in the meantime by #1443. As @sacrosanctic points out, data-sveltekit-reload shouldn't be automatically allowed.

So I think this can be closed, right? Thank you for it nonetheless, I used parts of this in #1433 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants