diff --git a/crates/oxc_linter/src/rules/nextjs/no_html_link_for_pages.rs b/crates/oxc_linter/src/rules/nextjs/no_html_link_for_pages.rs index 758853e0feb50..01fda574fd829 100644 --- a/crates/oxc_linter/src/rules/nextjs/no_html_link_for_pages.rs +++ b/crates/oxc_linter/src/rules/nextjs/no_html_link_for_pages.rs @@ -1,6 +1,6 @@ use oxc_ast::{ AstKind, - ast::{JSXAttributeItem, JSXAttributeName, JSXElementName}, + ast::{JSXAttributeItem, JSXAttributeName, JSXAttributeValue, JSXElementName}, }; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; @@ -93,6 +93,36 @@ impl Rule for NoHtmlLinkForPages { return; } + // Skip if target="_blank" - these links intentionally open in new tabs + // and don't need client-side navigation + let has_target_blank = jsx_opening_element.attributes.iter().any(|attr| { + if let JSXAttributeItem::Attribute(attr) = attr + && let JSXAttributeName::Identifier(name) = &attr.name + && name.name == "target" + { + return attr.value.as_ref().is_some_and(|value| { + matches!(value, JSXAttributeValue::StringLiteral(str_lit) if str_lit.value == "_blank") + }); + } + false + }); + + if has_target_blank { + return; + } + + // Skip if download attribute is present - these are file downloads, not navigation + let has_download_attr = jsx_opening_element.attributes.iter().any(|attr| { + matches!(attr, + JSXAttributeItem::Attribute(attr) + if matches!(&attr.name, JSXAttributeName::Identifier(name) if name.name == "download") + ) + }); + + if has_download_attr { + return; + } + // Find the href attribute let href_attr = jsx_opening_element.attributes.iter().find_map(|attr| { if let JSXAttributeItem::Attribute(attr) = attr @@ -109,15 +139,16 @@ impl Rule for NoHtmlLinkForPages { }; // Check if href value indicates an internal link + // Only check string literal hrefs - ignore expression hrefs (dynamic values) + // to match upstream behavior and avoid false positives let is_internal_link = href_attr.value.as_ref().is_some_and(|value| { match value { - // String literal href - oxc_ast::ast::JSXAttributeValue::StringLiteral(str_lit) => { + // String literal href - check if it's an internal link + JSXAttributeValue::StringLiteral(str_lit) => { let href_value = str_lit.value.as_str(); is_internal_page_link(href_value) } - // Expression href - we'll be conservative and flag it as potentially internal - oxc_ast::ast::JSXAttributeValue::ExpressionContainer(_) => true, + // Expression href (dynamic) - ignore, can't statically determine if internal _ => false, } }); @@ -215,18 +246,28 @@ fn test() { r"
Not an anchor
", // Next.js Link component (correct usage) r"About", + // Links with target="_blank" are allowed (opens in new tab, doesn't need client-side navigation) + r#"About"#, + r#"About"#, + r#"About"#, + // Download links are allowed (file downloads, not navigation) + r#"Download CSV"#, + r#"Download Report"#, + r#"Download JSON"#, + // Dynamic hrefs (expressions) are allowed - can't statically determine if internal + r"Dynamic", + r"User Profile", + r"Dynamic URL", ]; let fail = vec![ - // Internal page links + // Internal page links with string literals r"About", r"Contact Us", r"About", r"Contact", r"About", - // Dynamic hrefs (expressions) - r"Dynamic", - r"User Profile", + r"Home", ]; Tester::new(NoHtmlLinkForPages::NAME, NoHtmlLinkForPages::PLUGIN, pass, fail) diff --git a/crates/oxc_linter/src/snapshots/nextjs_no_html_link_for_pages.snap b/crates/oxc_linter/src/snapshots/nextjs_no_html_link_for_pages.snap index d44176e17b0d7..ad2a443d1eef8 100644 --- a/crates/oxc_linter/src/snapshots/nextjs_no_html_link_for_pages.snap +++ b/crates/oxc_linter/src/snapshots/nextjs_no_html_link_for_pages.snap @@ -1,6 +1,5 @@ --- source: crates/oxc_linter/src/tester.rs -assertion_line: 411 --- ⚠ eslint-plugin-next(no-html-link-for-pages): Do not use `` elements to navigate between Next.js pages. ╭─[no_html_link_for_pages.tsx:1:1] @@ -44,16 +43,8 @@ assertion_line: 411 ⚠ eslint-plugin-next(no-html-link-for-pages): Do not use `` elements to navigate between Next.js pages. ╭─[no_html_link_for_pages.tsx:1:1] - 1 │ Dynamic - · ───────────┬────────── - · ╰── Replace with `` from `next/link` - ╰──── - help: Use `` from `next/link` instead for internal navigation. See https://nextjs.org/docs/messages/no-html-link-for-pages - - ⚠ eslint-plugin-next(no-html-link-for-pages): Do not use `` elements to navigate between Next.js pages. - ╭─[no_html_link_for_pages.tsx:1:1] - 1 │ User Profile - · ──────────────┬───────────── - · ╰── Replace with `` from `next/link` + 1 │ Home + · ──────┬───── + · ╰── Replace with `` from `next/link` ╰──── help: Use `` from `next/link` instead for internal navigation. See https://nextjs.org/docs/messages/no-html-link-for-pages