Skip to content

Commit

Permalink
address PR review
Browse files Browse the repository at this point in the history
  • Loading branch information
@kaioduarte
kaioduarte committed Oct 16, 2024
1 parent c75d006 commit 62d87f8
Show file tree
Hide file tree
Showing 8 changed files with 223 additions and 59 deletions.
2 changes: 1 addition & 1 deletion crates/biome_configuration/src/analyzer/linter/rules.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3417,7 +3417,7 @@ pub struct Nursery {
#[doc = "Require explicit return types on functions and class methods."]
#[serde(skip_serializing_if = "Option::is_none")]
pub use_explicit_type: Option<RuleConfiguration<biome_js_analyze::options::UseExplicitType>>,
#[doc = "Enforces the use of a recommended font-display strategy with Google Fonts."]
#[doc = "Enforces the use of a recommended display strategy with Google Fonts."]
#[serde(skip_serializing_if = "Option::is_none")]
pub use_google_font_display:
Option<RuleConfiguration<biome_js_analyze::options::UseGoogleFontDisplay>>,
Expand Down
78 changes: 33 additions & 45 deletions crates/biome_js_analyze/src/lint/nursery/use_google_font_display.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ use biome_js_syntax::jsx_ext::AnyJsxElement;
use biome_rowan::TextRange;

declare_lint_rule! {
/// Enforces the use of a recommended `font-display` strategy with Google Fonts.
/// Enforces the use of a recommended `display` strategy with Google Fonts.
///
/// The `font-display` property controls how a font is displayed while it is loading. When using Google Fonts,
/// The `display` property controls how a font is displayed while it is loading. When using Google Fonts,
/// it's important to specify an appropriate value for this property to ensure good user experience and prevent layout shifts.
///
/// This rule flags the absence of the `font-display` parameter, or the usage of less optimal values such as `auto`, `block`, or `fallback`.
/// This rule flags the absence of the `display` parameter, or the usage of less optimal values such as `auto`, `block`, or `fallback`.
/// Using `&display=optional` is generally recommended as it minimizes the risk of invisible text or layout shifts.
/// In cases where swapping to the custom font after it has loaded is important, consider using `&display=swap`.
///
Expand Down Expand Up @@ -58,9 +58,16 @@ declare_lint_rule! {
}
}

const FORBIDDEN_VALUES: [&str; 3] = ["auto", "block", "fallback"];

pub enum FontDisplayIssue {
MissingDisplayParam,
ForbiddenValue,
}

impl Rule for UseGoogleFontDisplay {
type Query = Ast<AnyJsxElement>;
type State = (bool, TextRange);
type State = (FontDisplayIssue, TextRange);
type Signals = Option<Self::State>;
type Options = ();

Expand All @@ -71,67 +78,48 @@ impl Rule for UseGoogleFontDisplay {
return None;
}

let href = element.find_attribute_by_name("href");
let initializer = href?.initializer()?.value().ok()?;
let jsx_string = initializer.as_jsx_string();
let href_text = jsx_string?.inner_string_text().ok()?;
let href = element.find_attribute_by_name("href")?;
let initializer = href.initializer()?.value().ok()?.as_static_value()?;
let href_text = initializer.as_string_constant()?;

if !href_text.starts_with("https://fonts.googleapis.com/css") {
return None;
}

let display_param = href_text
.text()
.split('?')
.last()?
.split('&')
.find(|p| p.starts_with("display="));
let range = jsx_string?.value_token().ok()?.text_trimmed_range();
let range = initializer.range();

if let Some(display_param) = display_param {
if matches!(
&*display_param.replace("display=", ""),
"auto" | "block" | "fallback"
) {
return Some((false, range));
for forbidden_value in FORBIDDEN_VALUES {
if display_param.ends_with(forbidden_value) {
return Some((FontDisplayIssue::ForbiddenValue, range));
}
}
} else {
return Some((true, range));
return Some((FontDisplayIssue::MissingDisplayParam, range));
}

None
}

fn diagnostic(
_: &RuleContext<Self>,
(is_missing_param, range): &Self::State,
) -> Option<RuleDiagnostic> {
if *is_missing_param {
return Some(
RuleDiagnostic::new(
rule_category!(),
range,
markup! {
"The Google Font link is missing the "<Emphasis>"font-display"</Emphasis>" parameter."
},
)
.note(markup!{
"Add "<Emphasis>"&display=optional"</Emphasis>" to prevent invisible text and layout shifts. If font swapping is important, use "<Emphasis>"&display=swap"</Emphasis>"."
})
);
fn diagnostic(_: &RuleContext<Self>, (issue, range): &Self::State) -> Option<RuleDiagnostic> {
let title = match issue {
FontDisplayIssue::MissingDisplayParam => markup! {
"The Google Font link is missing the "<Emphasis>"display"</Emphasis>" parameter."
},
FontDisplayIssue::ForbiddenValue => markup! {
"The Google Font link has a non-recommended "<Emphasis>"display"</Emphasis>" value."
},
};

return Some(
RuleDiagnostic::new(
rule_category!(),
range,
markup! {
"The Google Font link has a non-recommended "<Emphasis>"font-display"</Emphasis>" value."
},
)
.note(markup!{
"Use "<Emphasis>"&display=optional"</Emphasis>" to improve performance and prevent layout shifts, or "<Emphasis>"&display=swap"</Emphasis>" if font swapping is necessary after loading."
})
);
Some(RuleDiagnostic::new(rule_category!(), range, title).note(
markup! {
"Use "<Emphasis>"&display=optional"</Emphasis>" to prevent invisible text and layout shifts. If font swapping is important, use "<Emphasis>"&display=swap"</Emphasis>"."
}
))
}
}
12 changes: 11 additions & 1 deletion crates/biome_js_analyze/tests/specs/nursery/useGoogleFontDisplay/invalid.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,14 @@
<link href="https://fonts.googleapis.com/css2?family=Krona+One&display=auto" />
<link href="https://fonts.googleapis.com/css2?family=Krona+One&display=block" />
<link href="https://fonts.googleapis.com/css2?family=Krona+One&display=fallback" />
</>

<link href={"https://fonts.googleapis.com/css2?family=Krona+One"} />
<link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=auto"} />
<link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=block"} />
<link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=fallback"} />

<link href={`https://fonts.googleapis.com/css2?family=Krona+One`} />
<link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=auto`} />
<link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=block`} />
<link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=fallback`} />
</>
166 changes: 156 additions & 10 deletions crates/biome_js_analyze/tests/specs/nursery/useGoogleFontDisplay/invalid.jsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,30 +10,41 @@ expression: invalid.jsx
<link href="https://fonts.googleapis.com/css2?family=Krona+One&display=auto" />
<link href="https://fonts.googleapis.com/css2?family=Krona+One&display=block" />
<link href="https://fonts.googleapis.com/css2?family=Krona+One&display=fallback" />

<link href={"https://fonts.googleapis.com/css2?family=Krona+One"} />
<link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=auto"} />
<link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=block"} />
<link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=fallback"} />

<link href={`https://fonts.googleapis.com/css2?family=Krona+One`} />
<link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=auto`} />
<link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=block`} />
<link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=fallback`} />
</>

```

# Diagnostics
```
invalid.jsx:2:13 lint/nursery/useGoogleFontDisplay ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The Google Font link is missing the font-display parameter.
! The Google Font link is missing the display parameter.
1 │ <>
> 2 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One" />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=auto" />
4 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=block" />
i Add &display=optional to prevent invisible text and layout shifts. If font swapping is important, use &display=swap.
i Use &display=optional to prevent invisible text and layout shifts. If font swapping is important, use &display=swap.
```

```
invalid.jsx:3:13 lint/nursery/useGoogleFontDisplay ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The Google Font link has a non-recommended font-display value.
! The Google Font link has a non-recommended display value.
1 │ <>
2 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One" />
Expand All @@ -42,40 +53,175 @@ invalid.jsx:3:13 lint/nursery/useGoogleFontDisplay ━━━━━━━━━
4 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=block" />
5 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=fallback" />
i Use &display=optional to improve performance and prevent layout shifts, or &display=swap if font swapping is necessary after loading.
i Use &display=optional to prevent invisible text and layout shifts. If font swapping is important, use &display=swap.
```

```
invalid.jsx:4:13 lint/nursery/useGoogleFontDisplay ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The Google Font link has a non-recommended font-display value.
! The Google Font link has a non-recommended display value.
2 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One" />
3 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=auto" />
> 4 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=block" />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=fallback" />
6 │ </>
6 │
i Use &display=optional to improve performance and prevent layout shifts, or &display=swap if font swapping is necessary after loading.
i Use &display=optional to prevent invisible text and layout shifts. If font swapping is important, use &display=swap.
```

```
invalid.jsx:5:13 lint/nursery/useGoogleFontDisplay ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The Google Font link has a non-recommended font-display value.
! The Google Font link has a non-recommended display value.
3 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=auto" />
4 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=block" />
> 5 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=fallback" />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6 │ </>
6 │
7 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One"} />
i Use &display=optional to prevent invisible text and layout shifts. If font swapping is important, use &display=swap.
```

```
invalid.jsx:7:14 lint/nursery/useGoogleFontDisplay ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The Google Font link is missing the display parameter.
5 │ <link href="https://fonts.googleapis.com/css2?family=Krona+One&display=fallback" />
6 │
> 7 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One"} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
8 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=auto"} />
9 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=block"} />
i Use &display=optional to prevent invisible text and layout shifts. If font swapping is important, use &display=swap.
```

```
invalid.jsx:8:14 lint/nursery/useGoogleFontDisplay ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The Google Font link has a non-recommended display value.
7 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One"} />
> 8 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=auto"} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
9 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=block"} />
10 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=fallback"} />
i Use &display=optional to prevent invisible text and layout shifts. If font swapping is important, use &display=swap.
```

```
invalid.jsx:9:14 lint/nursery/useGoogleFontDisplay ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The Google Font link has a non-recommended display value.
7 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One"} />
8 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=auto"} />
> 9 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=block"} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
10 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=fallback"} />
11 │
i Use &display=optional to prevent invisible text and layout shifts. If font swapping is important, use &display=swap.
```

```
invalid.jsx:10:14 lint/nursery/useGoogleFontDisplay ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The Google Font link has a non-recommended display value.
8 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=auto"} />
9 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=block"} />
> 10 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=fallback"} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11 │
12 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One`} />
i Use &display=optional to prevent invisible text and layout shifts. If font swapping is important, use &display=swap.
```

```
invalid.jsx:12:15 lint/nursery/useGoogleFontDisplay ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The Google Font link is missing the display parameter.
10 │ <link href={"https://fonts.googleapis.com/css2?family=Krona+One&display=fallback"} />
11 │
> 12 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One`} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=auto`} />
14 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=block`} />
i Use &display=optional to prevent invisible text and layout shifts. If font swapping is important, use &display=swap.
```

```
invalid.jsx:13:15 lint/nursery/useGoogleFontDisplay ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The Google Font link has a non-recommended display value.
12 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One`} />
> 13 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=auto`} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=block`} />
15 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=fallback`} />
i Use &display=optional to prevent invisible text and layout shifts. If font swapping is important, use &display=swap.
```

```
invalid.jsx:14:15 lint/nursery/useGoogleFontDisplay ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The Google Font link has a non-recommended display value.
12 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One`} />
13 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=auto`} />
> 14 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=block`} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=fallback`} />
16 │ </>
i Use &display=optional to prevent invisible text and layout shifts. If font swapping is important, use &display=swap.
```

```
invalid.jsx:15:15 lint/nursery/useGoogleFontDisplay ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The Google Font link has a non-recommended display value.
13 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=auto`} />
14 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=block`} />
> 15 │ <link href={`https://fonts.googleapis.com/css2?family=Krona+One&display=fallback`} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
16 │ </>
17 │
i Use &display=optional to improve performance and prevent layout shifts, or &display=swap if font swapping is necessary after loading.
i Use &display=optional to prevent invisible text and layout shifts. If font swapping is important, use &display=swap.
```
10 changes: 10 additions & 0 deletions crates/biome_js_analyze/tests/specs/nursery/useGoogleFontDisplay/valid.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,16 @@
href="https://fonts.googleapis.com/css2?family=Krona+One&display=optional"
rel="stylesheet"
/>
<link
href={"https://fonts.googleapis.com/css2?family=Krona+One&display=optional"}
rel="stylesheet"
/>
<link
href={`https://fonts.googleapis.com/css2?family=Krona+One&display=optional`}
rel="stylesheet"
/>
<link href="https://fonts.googleapis.com/css2?display=unknown" rel="stylesheet" />
<link href={"https://fonts.googleapis.com/css2?display=unknown"} rel="stylesheet" />
<link href={`https://fonts.googleapis.com/css2?display=unknown`} rel="stylesheet" />
<link rel="stylesheet" />
</>
Loading

0 comments on commit 62d87f8

Please sign in to comment.