From 812ea37153342034a7a558ed6bb094243503419f Mon Sep 17 00:00:00 2001 From: kaykdm Date: Mon, 29 Jan 2024 19:32:43 +0900 Subject: [PATCH 1/2] feat(linter): implement @next/next/no-before-interactive-script-outside-document --- crates/oxc_linter/src/rules.rs | 2 + ...ore_interactive_script_outside_document.rs | 394 ++++++++++++++++++ ...e_interactive_script_outside_document.snap | 59 +++ 3 files changed, 455 insertions(+) create mode 100644 crates/oxc_linter/src/rules/nextjs/no_before_interactive_script_outside_document.rs create mode 100644 crates/oxc_linter/src/snapshots/no_before_interactive_script_outside_document.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index ace2312e0385f..0c0422b0b1bde 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -299,6 +299,7 @@ mod nextjs { pub mod next_script_for_ga; pub mod no_assign_module_variable; pub mod no_async_client_component; + pub mod no_before_interactive_script_outside_document; pub mod no_css_tags; pub mod no_document_import_in_page; pub mod no_head_element; @@ -584,4 +585,5 @@ oxc_macros::declare_all_lint_rules! { nextjs::no_typos, nextjs::no_document_import_in_page, nextjs::no_unwanted_polyfillio, + nextjs::no_before_interactive_script_outside_document, } diff --git a/crates/oxc_linter/src/rules/nextjs/no_before_interactive_script_outside_document.rs b/crates/oxc_linter/src/rules/nextjs/no_before_interactive_script_outside_document.rs new file mode 100644 index 0000000000000..f1e6379823a1b --- /dev/null +++ b/crates/oxc_linter/src/rules/nextjs/no_before_interactive_script_outside_document.rs @@ -0,0 +1,394 @@ +use oxc_ast::{ + ast::{JSXAttributeItem, JSXAttributeName, JSXAttributeValue, JSXElementName, JSXIdentifier}, + AstKind, +}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint-plugin-next(no-before-interactive-script-outside-document): next/script's `beforeInteractive` strategy should not be used outside of `pages/_document.js`")] +#[diagnostic( + severity(warning), + help("See https://nextjs.org/docs/messages/no-before-interactive-script-outside-document") +)] +struct NoBeforeInteractiveScriptOutsideDocumentDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct NoBeforeInteractiveScriptOutsideDocument; + +declare_oxc_lint!( + /// ### What it does + /// Prevent usage of `next/script`'s `beforeInteractive` strategy outside of `pages/_document.js`. + /// + /// ### Why is this bad? + /// + /// + /// ### Example + /// ```javascript + /// ``` + NoBeforeInteractiveScriptOutsideDocument, + correctness +); + +impl Rule for NoBeforeInteractiveScriptOutsideDocument { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + if let AstKind::JSXOpeningElement(jsx_el) = node.kind() { + let Some(path) = ctx.file_path().to_str() else { return }; + let is_in_app_dir = path.contains("app/") || path.contains("app\\"); + if is_in_app_dir { + return; + } + + let Some(page) = path.split("pages").last() else { return }; + if page.starts_with("/_document") || page.starts_with("\\_document") { + return; + } + let JSXElementName::Identifier(JSXIdentifier { name: tag_name, .. }) = &jsx_el.name + else { + return; + }; + if jsx_el.attributes.len() == 0 { + return; + } + + let Some(JSXAttributeItem::Attribute(strategy)) = + jsx_el.attributes.iter().find(|attr| { + matches!( + attr, + JSXAttributeItem::Attribute(jsx_attr) + if matches!( + &jsx_attr.name, + JSXAttributeName::Identifier(id) if id.name.as_str() == "strategy" + ) + ) + }) + else { + return; + }; + + if let Some(JSXAttributeValue::StringLiteral(strategy_value)) = &strategy.value { + if strategy_value.value.as_str() == "beforeInteractive" { + let next_script_import_local_name = + ctx.semantic().module_record().import_entries.iter().find_map(|entry| { + if entry.module_request.name().as_str() == "next/script" { + Some(entry.local_name.name()) + } else { + None + } + }); + if !matches!(next_script_import_local_name, Some(import) if tag_name.as_str() == import.as_str()) + { + return; + } + ctx.diagnostic(NoBeforeInteractiveScriptOutsideDocumentDiagnostic( + strategy.span, + )); + } + } + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + use std::path::PathBuf; + + let pass = vec![ + ( + r#"import Document, { Html, Main, NextScript } from 'next/document' + import Script from 'next/script' + + class MyDocument extends Document { + render() { + return ( + + + + + +
+ + + + + ) + } + } + + export default MyDocument + "#, + None, + None, + Some(PathBuf::from("pages/_document.js")), + ), + ( + r#"import Document, { Html, Main, NextScript } from 'next/document' + import ScriptComponent from 'next/script' + + class MyDocument extends Document { + render() { + return ( + + + + + +
+ + + + + ) + } + } + + export default MyDocument + "#, + None, + None, + Some(PathBuf::from("pages/_document.tsx")), + ), + ( + r#"import Document, { Html, Main, NextScript } from 'next/document' + import ScriptComponent from 'next/script' + + class MyDocument extends Document { + render() { + return ( + + + + + +
+ + + + + ) + } + } + + export default MyDocument + "#, + None, + None, + Some(PathBuf::from("pages/_document.tsx")), + ), + ( + r#"import Script from "next/script"; + + export default function Index() { + return ( + + {children} + + ); + } + "#, + None, + None, + Some(PathBuf::from("pages/index.js")), + ), + ( + r#" import Head from "next/head"; + import Script from "next/script"; + + export default function Index() { + return ( + + ); + } + "#, + None, + None, + Some(PathBuf::from("components/outside-known-dirs.js")), + ), + ( + r#" import Script from "next/script"; + + export default function Index() { + return ( + + {children} + + ╰──── + help: See https://nextjs.org/docs/messages/no-before-interactive-script-outside-document + + ⚠ eslint-plugin-next(no-before-interactive-script-outside-document): next/script's `beforeInteractive` strategy should not be used outside of `pages/_document.js` + ╭─[no_before_interactive_script_outside_document.tsx:8:1] + 8 │ src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=scriptBeforeInteractive" + 9 │ strategy="beforeInteractive" + · ──────────────────────────── + 10 │ > + ╰──── + help: See https://nextjs.org/docs/messages/no-before-interactive-script-outside-document + + ⚠ eslint-plugin-next(no-before-interactive-script-outside-document): next/script's `beforeInteractive` strategy should not be used outside of `pages/_document.js` + ╭─[no_before_interactive_script_outside_document.tsx:8:1] + 8 │ src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=scriptBeforeInteractive" + 9 │ strategy='beforeInteractive' + · ──────────────────────────── + 10 │ /> + ╰──── + help: See https://nextjs.org/docs/messages/no-before-interactive-script-outside-document + + ⚠ eslint-plugin-next(no-before-interactive-script-outside-document): next/script's `beforeInteractive` strategy should not be used outside of `pages/_document.js` + ╭─[no_before_interactive_script_outside_document.tsx:8:1] + 8 │ src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=scriptBeforeInteractive" + 9 │ strategy='beforeInteractive' + · ──────────────────────────── + 10 │ /> + ╰──── + help: See https://nextjs.org/docs/messages/no-before-interactive-script-outside-document + + ⚠ eslint-plugin-next(no-before-interactive-script-outside-document): next/script's `beforeInteractive` strategy should not be used outside of `pages/_document.js` + ╭─[no_before_interactive_script_outside_document.tsx:8:1] + 8 │ src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=scriptBeforeInteractive" + 9 │ strategy='beforeInteractive' + · ──────────────────────────── + 10 │ /> + ╰──── + help: See https://nextjs.org/docs/messages/no-before-interactive-script-outside-document + + ⚠ eslint-plugin-next(no-before-interactive-script-outside-document): next/script's `beforeInteractive` strategy should not be used outside of `pages/_document.js` + ╭─[no_before_interactive_script_outside_document.tsx:8:1] + 8 │ src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js?a=scriptBeforeInteractive" + 9 │ strategy='beforeInteractive' + · ──────────────────────────── + 10 │ /> + ╰──── + help: See https://nextjs.org/docs/messages/no-before-interactive-script-outside-document + + From 3586a6a2c01402b02128e34973261eee76416147 Mon Sep 17 00:00:00 2001 From: kaykdm Date: Mon, 29 Jan 2024 21:54:54 +0900 Subject: [PATCH 2/2] Create nextjs util --- ...ore_interactive_script_outside_document.rs | 29 ++++++++----------- .../nextjs/no_document_import_in_page.rs | 9 +++--- .../src/rules/nextjs/no_head_element.rs | 7 ++--- .../rules/nextjs/no_unwanted_polyfillio.rs | 11 ++----- crates/oxc_linter/src/utils/mod.rs | 3 +- crates/oxc_linter/src/utils/nextjs.rs | 22 ++++++++++++++ 6 files changed, 45 insertions(+), 36 deletions(-) create mode 100644 crates/oxc_linter/src/utils/nextjs.rs diff --git a/crates/oxc_linter/src/rules/nextjs/no_before_interactive_script_outside_document.rs b/crates/oxc_linter/src/rules/nextjs/no_before_interactive_script_outside_document.rs index f1e6379823a1b..04c8c95fb2720 100644 --- a/crates/oxc_linter/src/rules/nextjs/no_before_interactive_script_outside_document.rs +++ b/crates/oxc_linter/src/rules/nextjs/no_before_interactive_script_outside_document.rs @@ -9,7 +9,12 @@ use oxc_diagnostics::{ use oxc_macros::declare_oxc_lint; use oxc_span::Span; -use crate::{context::LintContext, rule::Rule, AstNode}; +use crate::{ + context::LintContext, + rule::Rule, + utils::{get_next_script_import_local_name, is_document_page, is_in_app_dir}, + AstNode, +}; #[derive(Debug, Error, Diagnostic)] #[error("eslint-plugin-next(no-before-interactive-script-outside-document): next/script's `beforeInteractive` strategy should not be used outside of `pages/_document.js`")] @@ -39,14 +44,8 @@ declare_oxc_lint!( impl Rule for NoBeforeInteractiveScriptOutsideDocument { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { if let AstKind::JSXOpeningElement(jsx_el) = node.kind() { - let Some(path) = ctx.file_path().to_str() else { return }; - let is_in_app_dir = path.contains("app/") || path.contains("app\\"); - if is_in_app_dir { - return; - } - - let Some(page) = path.split("pages").last() else { return }; - if page.starts_with("/_document") || page.starts_with("\\_document") { + let Some(file_path) = ctx.file_path().to_str() else { return }; + if is_in_app_dir(file_path) { return; } let JSXElementName::Identifier(JSXIdentifier { name: tag_name, .. }) = &jsx_el.name @@ -74,14 +73,10 @@ impl Rule for NoBeforeInteractiveScriptOutsideDocument { if let Some(JSXAttributeValue::StringLiteral(strategy_value)) = &strategy.value { if strategy_value.value.as_str() == "beforeInteractive" { - let next_script_import_local_name = - ctx.semantic().module_record().import_entries.iter().find_map(|entry| { - if entry.module_request.name().as_str() == "next/script" { - Some(entry.local_name.name()) - } else { - None - } - }); + if is_document_page(file_path) { + return; + } + let next_script_import_local_name = get_next_script_import_local_name(ctx); if !matches!(next_script_import_local_name, Some(import) if tag_name.as_str() == import.as_str()) { return; diff --git a/crates/oxc_linter/src/rules/nextjs/no_document_import_in_page.rs b/crates/oxc_linter/src/rules/nextjs/no_document_import_in_page.rs index 7f6f3c115eae0..eafb97a97cb8d 100644 --- a/crates/oxc_linter/src/rules/nextjs/no_document_import_in_page.rs +++ b/crates/oxc_linter/src/rules/nextjs/no_document_import_in_page.rs @@ -6,7 +6,7 @@ use oxc_diagnostics::{ use oxc_macros::declare_oxc_lint; use oxc_span::Span; -use crate::{context::LintContext, rule::Rule, AstNode}; +use crate::{context::LintContext, rule::Rule, utils::is_document_page, AstNode}; #[derive(Debug, Error, Diagnostic)] #[error("eslint-plugin-next(no-document-import-in-page): `` from `next/document` should not be imported outside of `pages/_document.js`. See: https://nextjs.org/docs/messages/no-document-import-in-page")] @@ -46,9 +46,8 @@ impl Rule for NoDocumentImportInPage { } let Some(path) = ctx.file_path().to_str() else { return }; - let Some(page) = path.split("pages").last() else { return }; - if page.starts_with("/_document") || page.starts_with("\\_document") { + if is_document_page(path) { return; } @@ -64,7 +63,7 @@ fn test() { let pass = vec![ ( r#"import Document from "next/document" - + export default class MyDocument extends Document { render() { return ( @@ -80,7 +79,7 @@ fn test() { ), ( r#"import Document from "next/document" - + export default class MyDocument extends Document { render() { return ( diff --git a/crates/oxc_linter/src/rules/nextjs/no_head_element.rs b/crates/oxc_linter/src/rules/nextjs/no_head_element.rs index 6811475fc12aa..a75cecc72e31a 100644 --- a/crates/oxc_linter/src/rules/nextjs/no_head_element.rs +++ b/crates/oxc_linter/src/rules/nextjs/no_head_element.rs @@ -6,7 +6,7 @@ use oxc_diagnostics::{ use oxc_macros::declare_oxc_lint; use oxc_span::Span; -use crate::{context::LintContext, rule::Rule, AstNode}; +use crate::{context::LintContext, rule::Rule, utils::is_in_app_dir, AstNode}; #[derive(Debug, Error, Diagnostic)] #[error("eslint-plugin-next(no-head-element): Do not use `` element. Use `` from `next/head` instead.")] @@ -33,8 +33,7 @@ declare_oxc_lint!( impl Rule for NoHeadElement { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { let Some(full_file_path) = ctx.file_path().to_str() else { return }; - let is_in_app_dir = full_file_path.contains("app/") || full_file_path.contains("app\\"); - if is_in_app_dir { + if is_in_app_dir(full_file_path) { return; } if let AstKind::JSXOpeningElement(elem) = node.kind() { @@ -54,7 +53,7 @@ fn test() { let pass = vec![ ( r"import Head from 'next/head'; - + export class MyComponent { render() { return ( diff --git a/crates/oxc_linter/src/rules/nextjs/no_unwanted_polyfillio.rs b/crates/oxc_linter/src/rules/nextjs/no_unwanted_polyfillio.rs index cdff0ddc3b4c4..235ac62338a81 100644 --- a/crates/oxc_linter/src/rules/nextjs/no_unwanted_polyfillio.rs +++ b/crates/oxc_linter/src/rules/nextjs/no_unwanted_polyfillio.rs @@ -11,7 +11,7 @@ use oxc_semantic::AstNode; use oxc_span::Span; use phf::{phf_set, Set}; -use crate::{context::LintContext, rule::Rule}; +use crate::{context::LintContext, rule::Rule, utils::get_next_script_import_local_name}; #[derive(Debug, Error, Diagnostic)] #[error("eslint-plugin-next(no-unwanted-polyfillio): No duplicate polyfills from Polyfill.io are allowed. {0} already shipped with Next.js.")] @@ -117,14 +117,7 @@ impl Rule for NoUnwantedPolyfillio { }; if tag_name.as_str() != "script" { - let next_script_import_local_name = - ctx.semantic().module_record().import_entries.iter().find_map(|entry| { - if entry.module_request.name().as_str() == "next/script" { - Some(entry.local_name.name()) - } else { - None - } - }); + let next_script_import_local_name = get_next_script_import_local_name(ctx); if !matches!(next_script_import_local_name, Some(import) if tag_name.as_str() == import.as_str()) { return; diff --git a/crates/oxc_linter/src/utils/mod.rs b/crates/oxc_linter/src/utils/mod.rs index 117b892b102bc..bfbd1e9ec8266 100644 --- a/crates/oxc_linter/src/utils/mod.rs +++ b/crates/oxc_linter/src/utils/mod.rs @@ -1,7 +1,8 @@ mod jest; +mod nextjs; mod node; mod react; mod react_perf; mod unicorn; -pub use self::{jest::*, node::*, react::*, react_perf::*, unicorn::*}; +pub use self::{jest::*, nextjs::*, node::*, react::*, react_perf::*, unicorn::*}; diff --git a/crates/oxc_linter/src/utils/nextjs.rs b/crates/oxc_linter/src/utils/nextjs.rs new file mode 100644 index 0000000000000..e3e26cceec40a --- /dev/null +++ b/crates/oxc_linter/src/utils/nextjs.rs @@ -0,0 +1,22 @@ +use oxc_span::Atom; + +use crate::LintContext; + +pub fn is_in_app_dir(file_path: &str) -> bool { + file_path.contains("app/") || file_path.contains("app\\") +} + +pub fn is_document_page(file_path: &str) -> bool { + let Some(page) = file_path.split("pages").last() else { return false }; + page.starts_with("/_document") || page.starts_with("\\_document") +} + +pub fn get_next_script_import_local_name<'a>(ctx: &'a LintContext) -> Option<&'a Atom> { + ctx.semantic().module_record().import_entries.iter().find_map(|entry| { + if entry.module_request.name().as_str() == "next/script" { + Some(entry.local_name.name()) + } else { + None + } + }) +}