-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(biome_js_analyze): port noBeforeInteractiveScriptOutsideDocument from Next.js
#8580
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
19e9e7a
6652dc5
84eda87
41e59f5
7456dfa
0d45f0a
0735b7c
f568b8b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@biomejs/biome": patch | ||
|
|
||
| Added the nursery rule [`noBeforeInteractiveScriptOutsideDocument`](https://biomejs.dev/linter/rules/no-before-interactive-script-outside-document/) to the Next.js domain. | ||
| This rule prevents usage of `next/script`'s `beforeInteractive` strategy outside of `pages/_document.js`. |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| use biome_analyze::{ | ||
| Rule, RuleDiagnostic, RuleDomain, RuleSource, context::RuleContext, declare_lint_rule, | ||
| }; | ||
| use biome_console::markup; | ||
| use biome_diagnostics::Severity; | ||
| use biome_js_syntax::jsx_ext::AnyJsxElement; | ||
| use biome_rowan::{AstNode, TextRange}; | ||
| use biome_rule_options::no_before_interactive_script_outside_document::NoBeforeInteractiveScriptOutsideDocumentOptions; | ||
|
|
||
| use crate::{ | ||
| nextjs::{NextUtility, is_next_import}, | ||
| services::semantic::Semantic, | ||
| }; | ||
|
|
||
| declare_lint_rule! { | ||
| /// Prevent usage of `next/script`'s `beforeInteractive` strategy outside of `pages/_document.js` in a Next.js project. | ||
| /// | ||
| /// Next.js provides a `<Script>` component from `next/script` to optimize the loading of third-party scripts. Using the `beforeInteractive` | ||
| /// strategy allows scripts to be preloaded before any first-party code. `beforeInteractive` scripts must be placed in `pages/_document.js`. | ||
| /// | ||
| /// This rule checks for any usage of the `beforeInteractive` scripts outside of these files. | ||
| /// | ||
| /// ## Examples | ||
| /// | ||
| /// ### Invalid | ||
| /// | ||
| /// ```jsx,expect_diagnostic | ||
| /// // pages/index.jsx | ||
| /// import Script from 'next/script' | ||
| /// | ||
| /// export default function Index() { | ||
| /// return ( | ||
| /// <div> | ||
| /// <Script | ||
| /// src="https://example.com/script.js" | ||
| /// strategy="beforeInteractive" | ||
| /// ></Script> | ||
| /// </div> | ||
| /// ) | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// ### Valid | ||
| /// | ||
| /// ```jsx,ignore | ||
| /// // pages/_document.jsx | ||
| /// import { Html, Head, Main, NextScript } from 'next/document' | ||
| /// import Script from 'next/script' | ||
| /// | ||
| /// export default function Document() { | ||
| /// return ( | ||
| /// <Html> | ||
| /// <Head /> | ||
| /// <body> | ||
| /// <Main /> | ||
| /// <NextScript /> | ||
| /// <Script | ||
| /// src="https://example.com/script.js" | ||
| /// strategy="beforeInteractive" | ||
| /// ></Script> | ||
| /// </body> | ||
| /// </Html> | ||
| /// ) | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| pub NoBeforeInteractiveScriptOutsideDocument { | ||
| version: "next", | ||
| name: "noBeforeInteractiveScriptOutsideDocument", | ||
| language: "jsx", | ||
| sources: &[RuleSource::EslintNext("no-before-interactive-script-outside-document").same()], | ||
| recommended: false, | ||
| severity: Severity::Warning, | ||
| domains: &[RuleDomain::Next], | ||
| } | ||
| } | ||
|
|
||
| impl Rule for NoBeforeInteractiveScriptOutsideDocument { | ||
| type Query = Semantic<AnyJsxElement>; | ||
| type State = TextRange; | ||
| type Signals = Option<Self::State>; | ||
| type Options = NoBeforeInteractiveScriptOutsideDocumentOptions; | ||
|
|
||
| fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
| let is_in_app_dir = ctx | ||
| .file_path() | ||
| .ancestors() | ||
| .any(|a| a.file_name().is_some_and(|f| f == "app" && a.is_dir())); | ||
| // should not run in app dir | ||
| if is_in_app_dir { | ||
| return None; | ||
| } | ||
|
|
||
| let jsx_element = ctx.query(); | ||
| let element_name = jsx_element.name().ok()?.name_value_token().ok()?; | ||
| if element_name.text_trimmed() != "Script" { | ||
| return None; | ||
| } | ||
|
|
||
| let semantic_model = ctx.model(); | ||
| let reference = jsx_element.name().ok()?; | ||
| let reference = reference.as_jsx_reference_identifier()?; | ||
| let binding = semantic_model.binding(reference)?; | ||
| if !is_next_import(&binding, NextUtility::Script) { | ||
| return None; | ||
| } | ||
|
|
||
| let strategy_attribute = jsx_element.find_attribute_by_name("strategy")?; | ||
| let strategy_attribute_value = strategy_attribute.as_static_value()?; | ||
| let strategy_attribute_value = strategy_attribute_value.text(); | ||
| if strategy_attribute_value != "beforeInteractive" { | ||
| return None; | ||
| } | ||
|
|
||
| let path = ctx.file_path(); | ||
|
|
||
| let file_name = path.file_stem()?; | ||
|
|
||
| // pages/_document.(js|ts|jsx|tsx) | ||
| let is_in_pages_dir = path.parent()?.file_name().is_some_and(|f| f == "pages"); | ||
| if is_in_pages_dir && file_name == "_document" { | ||
| return None; | ||
| } | ||
|
|
||
| Some(jsx_element.syntax().text_range_with_trivia()) | ||
| } | ||
|
|
||
| fn diagnostic(_: &RuleContext<Self>, range: &Self::State) -> Option<RuleDiagnostic> { | ||
| Some(RuleDiagnostic::new( | ||
| rule_category!(), | ||
| range, | ||
| markup! { | ||
| "Don't use "<Emphasis>"next/script"</Emphasis>" component with the `"<Emphasis>"beforeInteractive"</Emphasis>"` strategy outside of "<Emphasis>"pages/_document.js"</Emphasis>"." | ||
| }, | ||
| ).note(markup! { | ||
| "See the "<Hyperlink href="https://nextjs.org/docs/messages/no-before-interactive-script-outside-document">"Next.js docs"</Hyperlink>" for more details." | ||
| })) | ||
| } | ||
| } | ||
|
dyc3 marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| /* should not generate diagnostics */ | ||
| import Script from 'next/script' | ||
|
|
||
| export default function RootLayout({ children }) { | ||
| return ( | ||
| <html lang="en"> | ||
| <body>{children}</body> | ||
| <Script | ||
| src="https://example.com/script.js" | ||
| strategy="beforeInteractive" | ||
| /> | ||
| </html> | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: layout.jsx | ||
| --- | ||
| # Input | ||
| ```jsx | ||
| /* should not generate diagnostics */ | ||
| import Script from 'next/script' | ||
|
|
||
| export default function RootLayout({ children }) { | ||
| return ( | ||
| <html lang="en"> | ||
| <body>{children}</body> | ||
| <Script | ||
| src="https://example.com/script.js" | ||
| strategy="beforeInteractive" | ||
| /> | ||
| </html> | ||
| ) | ||
| } | ||
|
|
||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| /* should not generate diagnostics */ | ||
| import { Html, Head, Main, NextScript } from 'next/document' | ||
| import Script from 'next/script' | ||
|
|
||
| export default function Document() { | ||
| return ( | ||
| <Html> | ||
| <Head /> | ||
| <body> | ||
| <Main /> | ||
| <NextScript /> | ||
| <Script | ||
| src="https://example.com/script.js" | ||
| strategy="beforeInteractive" | ||
| ></Script> | ||
| </body> | ||
| </Html> | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: _document.jsx | ||
| --- | ||
| # Input | ||
| ```jsx | ||
| /* should not generate diagnostics */ | ||
| import { Html, Head, Main, NextScript } from 'next/document' | ||
| import Script from 'next/script' | ||
|
|
||
| export default function Document() { | ||
| return ( | ||
| <Html> | ||
| <Head /> | ||
| <body> | ||
| <Main /> | ||
| <NextScript /> | ||
| <Script | ||
| src="https://example.com/script.js" | ||
| strategy="beforeInteractive" | ||
| ></Script> | ||
| </body> | ||
| </Html> | ||
| ) | ||
| } | ||
|
|
||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| /* should generate diagnostics */ | ||
| import Script from 'next/script' | ||
|
|
||
| export default function Index() { | ||
| return ( | ||
| <div> | ||
| <Script | ||
| src="https://example.com/script.js" | ||
| strategy="beforeInteractive" | ||
| ></Script> | ||
| </div> | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: index.jsx | ||
| --- | ||
| # Input | ||
| ```jsx | ||
| /* should generate diagnostics */ | ||
| import Script from 'next/script' | ||
|
|
||
| export default function Index() { | ||
| return ( | ||
| <div> | ||
| <Script | ||
| src="https://example.com/script.js" | ||
| strategy="beforeInteractive" | ||
| ></Script> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| ``` | ||
|
|
||
| # Diagnostics | ||
| ``` | ||
| index.jsx:7:7 lint/nursery/noBeforeInteractiveScriptOutsideDocument ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| ! Don't use next/script component with the `beforeInteractive` strategy outside of pages/_document.js. | ||
|
|
||
| 5 │ return ( | ||
| 6 │ <div> | ||
| > 7 │ <Script | ||
| │ ^^^^^^^ | ||
| > 8 │ src="https://example.com/script.js" | ||
| > 9 │ strategy="beforeInteractive" | ||
| > 10 │ ></Script> | ||
| │ ^ | ||
| 11 │ </div> | ||
| 12 │ ) | ||
|
|
||
| i See the Next.js docs for more details. | ||
|
|
||
| i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. | ||
|
|
||
|
|
||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| use biome_deserialize_macros::{Deserializable, Merge}; | ||
| use serde::{Deserialize, Serialize}; | ||
| #[derive(Default, Clone, Debug, Deserialize, Deserializable, Merge, Eq, PartialEq, Serialize)] | ||
| #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields, default)] | ||
| pub struct NoBeforeInteractiveScriptOutsideDocumentOptions {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential false negative for files under unrelated
appdirectories.The ancestor check matches any directory named
appin the path, not just Next.js's App Router directory. A path like/home/user/app/my-project/pages/index.jsxwould incorrectly skip the rule.Consider checking that
appis a sibling topagesor at a known project root level, though this edge case may be acceptable for practical use.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is hard for me to cover these edge cases just by file path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can always straighten those out later