-
-
Notifications
You must be signed in to change notification settings - Fork 809
feat(biome_js_analyze): port useInlineScriptId from Next.js
#8624
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
base: main
Are you sure you want to change the base?
Changes from all commits
0a70328
b76cace
c0a5bb1
9ecbc8a
0588d1c
73d2378
9ab2f3d
cd3ab77
358e900
4b10833
bcff277
1f25891
3743340
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,18 @@ | ||
| --- | ||
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Added the nursery rule [`useInlineScriptId`](https://biomejs.dev/linter/rules/use-inline-script-id/) to the Next.js domain. | ||
| This rule enforces `id` attribute on `next/script` components with inline content or `dangerouslySetInnerHTML`. | ||
|
|
||
| The following code is invalid: | ||
|
|
||
| ```jsx | ||
| import Script from 'next/script'; | ||
|
|
||
| export default function Page() { | ||
| return ( | ||
| <Script>{`console.log('Hello');`}</Script> // must have `id` attribute | ||
| ); | ||
| } | ||
| ``` | ||
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.
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,183 @@ | ||
| use biome_analyze::{ | ||
| Rule, RuleDiagnostic, RuleDomain, RuleSource, context::RuleContext, declare_lint_rule, | ||
| }; | ||
| use biome_console::markup; | ||
| use biome_diagnostics::Severity; | ||
| use biome_js_syntax::{ | ||
| AnyJsExpression, AnyJsxAttribute, JsObjectExpression, JsVariableDeclarator, JsxElement, | ||
| jsx_ext::AnyJsxElement, | ||
| }; | ||
| use biome_rowan::{AstNode, AstNodeList, TextRange, TokenText}; | ||
| use biome_rule_options::use_inline_script_id::UseInlineScriptIdOptions; | ||
| use rustc_hash::FxHashSet; | ||
|
|
||
| use crate::{ | ||
| nextjs::{NextUtility, is_next_import}, | ||
| services::semantic::Semantic, | ||
| }; | ||
|
|
||
| declare_lint_rule! { | ||
| /// Enforce `id` attribute on `next/script` components with inline content or `dangerouslySetInnerHTML`. | ||
| /// | ||
| /// Using inline scripts or `dangerouslySetInnerHTML` in `next/script` components requires an `id` attribute to ensure that Next.js can track and optimize them correctly. | ||
| /// | ||
| /// ## Examples | ||
| /// | ||
| /// ### Invalid | ||
| /// | ||
| /// ```jsx,expect_diagnostic | ||
| /// import Script from 'next/script' | ||
| /// | ||
| /// export default function Page() { | ||
| /// return ( | ||
| /// <Script>{`console.log('Hello world!');`}</Script> | ||
| /// ) | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// ```jsx,expect_diagnostic | ||
| /// import Script from 'next/script' | ||
| /// | ||
| /// export default function Page() { | ||
| /// return ( | ||
| /// <Script dangerouslySetInnerHTML={{ __html: `console.log('Hello world!');` }} /> | ||
| /// ) | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// ### Valid | ||
| /// ```jsx | ||
| /// import Script from 'next/script' | ||
| /// | ||
| /// export default function Page() { | ||
| /// return ( | ||
| /// <Script id="my-script">{`console.log('Hello world!');`}</Script> | ||
| /// ) | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// ```jsx | ||
| /// import Script from 'next/script' | ||
| /// | ||
| /// export default function Page() { | ||
| /// return ( | ||
| /// <Script id="my-script" dangerouslySetInnerHTML={{ __html: `console.log('Hello world!');` }} /> | ||
| /// ) | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| pub UseInlineScriptId { | ||
| version: "next", | ||
| name: "useInlineScriptId", | ||
| language: "jsx", | ||
| sources: &[RuleSource::EslintNext("inline-script-id").same()], | ||
| recommended: true, | ||
| severity: Severity::Error, | ||
| domains: &[RuleDomain::Next], | ||
| } | ||
| } | ||
|
|
||
| impl Rule for UseInlineScriptId { | ||
| type Query = Semantic<AnyJsxElement>; | ||
| type State = TextRange; | ||
| type Signals = Option<Self::State>; | ||
| type Options = UseInlineScriptIdOptions; | ||
|
|
||
| fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
| let jsx_element = ctx.query(); | ||
|
|
||
| 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 mut attribute_names = FxHashSet::default(); | ||
| for attribute in jsx_element.attributes() { | ||
| match attribute { | ||
| AnyJsxAttribute::JsxAttribute(a) => { | ||
| if let Ok(name_value) = a.name_value_token() { | ||
| let name = name_value.token_text(); | ||
| attribute_names.insert(name); | ||
| } | ||
| } | ||
| AnyJsxAttribute::JsxSpreadAttribute(spread) => { | ||
| if let Ok(argument) = spread.argument() { | ||
| match argument { | ||
| AnyJsExpression::JsObjectExpression(obj_expr) => { | ||
| collect_property_names(&obj_expr, &mut attribute_names)?; | ||
| } | ||
| AnyJsExpression::JsIdentifierExpression(ident_expr) => { | ||
| if let Ok(reference) = ident_expr.name() | ||
| && let Some(binding) = semantic_model.binding(&reference) | ||
| && let Some(declarator) = binding | ||
| .syntax() | ||
| .ancestors() | ||
| .find_map(JsVariableDeclarator::cast) | ||
| && let Some(initializer) = declarator.initializer() | ||
| && let Ok(expression) = initializer.expression() | ||
| && let AnyJsExpression::JsObjectExpression(obj_expr) = | ||
| expression | ||
| { | ||
| collect_property_names(&obj_expr, &mut attribute_names)?; | ||
| } | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
|
|
||
| let has_children = jsx_element | ||
| .parent::<JsxElement>() | ||
| .is_some_and(|parent| !parent.children().is_empty()); | ||
| if (has_children || attribute_names.contains("dangerouslySetInnerHTML")) | ||
| && !attribute_names.contains("id") | ||
| { | ||
| return Some(jsx_element.range()); | ||
| } | ||
|
|
||
| None | ||
| } | ||
|
|
||
| fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> { | ||
| Some( | ||
| RuleDiagnostic::new( | ||
| rule_category!(), | ||
| state, | ||
| markup! { | ||
| ""<Emphasis>"next/script"</Emphasis>" components have inline content or `dangerouslySetInnerHTML` without "<Emphasis>"id"</Emphasis>" attribute." | ||
| }, | ||
| ) | ||
| .note(markup!( | ||
| "Next.js requires "<Emphasis>"id"</Emphasis>" attribute to track and optimize inline scripts. Without it, performance issues may occur." | ||
| )) | ||
| .note(markup! { | ||
| "See the "<Hyperlink href="https://nextjs.org/docs/messages/inline-script-id">"Next.js docs"</Hyperlink>" for more details." | ||
| }) | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| fn collect_property_names( | ||
| obj_expr: &JsObjectExpression, | ||
| set: &mut FxHashSet<TokenText>, | ||
| ) -> Option<()> { | ||
| for member in obj_expr.members() { | ||
| let member = member.ok()?; | ||
| if let Some(property_member) = member.as_js_property_object_member() | ||
| && let Some(name) = property_member.name().ok().and_then(|n| n.name()) | ||
| { | ||
| set.insert(name); | ||
| } else if let Some(shorthand) = member.as_js_shorthand_property_object_member() | ||
| && let Some(name) = shorthand.name().ok().and_then(|n| n.name().ok()) | ||
| { | ||
| set.insert(name); | ||
| } | ||
| } | ||
| Some(()) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| /* should generate diagnostics */ | ||
| import Script from 'next/script' | ||
|
|
||
| export default function Page() { | ||
| return ( | ||
| <Script>{`console.log('Hello world!');`}</Script> | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: invalid-01.jsx | ||
| --- | ||
| # Input | ||
| ```jsx | ||
| /* should generate diagnostics */ | ||
| import Script from 'next/script' | ||
|
|
||
| export default function Page() { | ||
| return ( | ||
| <Script>{`console.log('Hello world!');`}</Script> | ||
| ) | ||
| } | ||
|
|
||
| ``` | ||
|
|
||
| # Diagnostics | ||
| ``` | ||
| invalid-01.jsx:6:5 lint/nursery/useInlineScriptId ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| × next/script components have inline content or `dangerouslySetInnerHTML` without id attribute. | ||
|
|
||
| 4 │ export default function Page() { | ||
| 5 │ return ( | ||
| > 6 │ <Script>{`console.log('Hello world!');`}</Script> | ||
| │ ^^^^^^^^ | ||
| 7 │ ) | ||
| 8 │ } | ||
|
|
||
| i Next.js requires id attribute to track and optimize inline scripts. Without it, performance issues may occur. | ||
|
|
||
| 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,8 @@ | ||
| /* should generate diagnostics */ | ||
| import Script from 'next/script' | ||
|
|
||
| export default function Page() { | ||
| return ( | ||
| <Script dangerouslySetInnerHTML={{ __html: `console.log('Hello world!');` }} /> | ||
| ) | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.