Skip to content

Commit 11c34e7

Browse files
committed
fix(linter/no-img-element): improve diagnostic and docs (#10908)
## What This PR Does Updates `eslint-next-plugin/no-img-element` to make diagnostic messages/labels/etc more informative and add documentation
1 parent bb999a3 commit 11c34e7

File tree

2 files changed

+93
-20
lines changed

2 files changed

+93
-20
lines changed

crates/oxc_linter/src/rules/nextjs/no_img_element.rs

Lines changed: 66 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,21 @@
1-
use oxc_ast::{AstKind, ast::JSXElementName};
1+
use oxc_ast::{
2+
AstKind,
3+
ast::{JSXAttributeItem, JSXElementName},
4+
};
25
use oxc_diagnostics::OxcDiagnostic;
36
use oxc_macros::declare_oxc_lint;
4-
use oxc_span::Span;
7+
use oxc_span::{GetSpan, Span};
58

69
use crate::{AstNode, context::LintContext, rule::Rule};
710

8-
fn no_img_element_diagnostic(span: Span) -> OxcDiagnostic {
9-
OxcDiagnostic::warn("Prevent usage of `<img>` element due to slower LCP and higher bandwidth.")
10-
.with_help("See https://nextjs.org/docs/messages/no-img-element")
11-
.with_label(span)
11+
fn no_img_element_diagnostic(span: Span, src_span: Option<Span>) -> OxcDiagnostic {
12+
let mut diagnostic = OxcDiagnostic::warn("Using `<img>` could result in slower LCP and higher bandwidth.")
13+
.with_help("Consider using `<Image />` from `next/image` or a custom image loader to automatically optimize images.\nSee https://nextjs.org/docs/messages/no-img-element")
14+
.with_label(span.label("Use `<Image />` from `next/image` instead."));
15+
if let Some(src_span) = src_span {
16+
diagnostic = diagnostic.and_label(src_span.label("Use a static image import instead."));
17+
}
18+
diagnostic
1219
}
1320

1421
#[derive(Debug, Default, Clone)]
@@ -17,16 +24,45 @@ pub struct NoImgElement;
1724
declare_oxc_lint!(
1825
/// ### What it does
1926
///
27+
/// Prevent the usage of `<img>` element due to slower
28+
/// [LCP](https://nextjs.org/learn/seo/lcp) and higher bandwidth.
2029
///
2130
/// ### Why is this bad?
2231
///
32+
/// `<img>` elements are not optimized for performance and can result in
33+
/// slower LCP and higher bandwidth. Using [`<Image />`](https://nextjs.org/docs/pages/api-reference/components/image)
34+
/// from `next/image` will automatically optimize images and serve them as
35+
/// static assets.
2336
///
2437
/// ### Example
38+
///
39+
/// Examples of **incorrect** code for this rule:
40+
/// ```javascript
41+
/// export function MyComponent() {
42+
/// return (
43+
/// <div>
44+
/// <img src="/test.png" alt="Test picture" />
45+
/// </div>
46+
/// );
47+
/// }
48+
/// ```
49+
///
50+
/// Examples of **correct** code for this rule:
2551
/// ```javascript
52+
/// import Image from "next/image";
53+
/// import testImage from "./test.png"
54+
/// export function MyComponent() {
55+
/// return (
56+
/// <div>
57+
/// <Image src={testImage} alt="Test picture" />
58+
/// </div>
59+
/// );
60+
/// }
2661
/// ```
2762
NoImgElement,
2863
nextjs,
29-
correctness
64+
correctness,
65+
pending // TODO: add `import Image from "next/image"` (if missing), then change `<img />` to `<Image />`
3066
);
3167

3268
impl Rule for NoImgElement {
@@ -39,18 +75,16 @@ impl Rule for NoImgElement {
3975
return;
4076
};
4177

42-
if jsx_opening_element_name.name.as_str() != "img" {
78+
if jsx_opening_element_name.name != "img" {
4379
return;
4480
}
4581

46-
let Some(parent) = ctx.nodes().parent_node(node.id()) else {
47-
return;
48-
};
49-
let Some(parent) = ctx.nodes().parent_node(parent.id()) else {
82+
// first two are self, parent. third is grandparent
83+
let Some(grandparent) = ctx.nodes().ancestor_kinds(node.id()).nth(2) else {
5084
return;
5185
};
5286

53-
if let AstKind::JSXElement(maybe_picture_jsx_elem) = parent.kind() {
87+
if let AstKind::JSXElement(maybe_picture_jsx_elem) = grandparent {
5488
if let JSXElementName::Identifier(jsx_opening_element_name) =
5589
&maybe_picture_jsx_elem.opening_element.name
5690
{
@@ -60,7 +94,18 @@ impl Rule for NoImgElement {
6094
}
6195
}
6296

63-
ctx.diagnostic(no_img_element_diagnostic(jsx_opening_element_name.span));
97+
let src_span: Option<Span> = jsx_opening_element
98+
.attributes
99+
.iter()
100+
.filter_map(JSXAttributeItem::as_attribute)
101+
.find_map(|attr| {
102+
let ident = attr.name.as_identifier()?;
103+
let value = attr.value.as_ref()?;
104+
let lit = value.as_string_literal()?;
105+
(ident.name == "src").then(|| lit.span())
106+
});
107+
108+
ctx.diagnostic(no_img_element_diagnostic(jsx_opening_element_name.span, src_span));
64109
}
65110
}
66111

@@ -146,6 +191,13 @@ fn test() {
146191
);
147192
}
148193
}"#,
194+
// src is not a string literal, so diagnostic won't label it.
195+
"
196+
import somePicture from './foo.png';
197+
export const MyComponent = () => (
198+
<img src={somePicture.src} alt='foo' />
199+
);
200+
",
149201
];
150202

151203
Tester::new(NoImgElement::NAME, NoImgElement::PLUGIN, pass, fail).test_and_snapshot();
Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,41 @@
11
---
22
source: crates/oxc_linter/src/tester.rs
33
---
4-
eslint-plugin-next(no-img-element): Prevent usage of `<img>` element due to slower LCP and higher bandwidth.
4+
eslint-plugin-next(no-img-element): Using `<img>` could result in slower LCP and higher bandwidth.
55
╭─[no_img_element.tsx:6:19]
66
5 │ <div>
77
6 │ <img
8-
· ───
8+
· ─┬─
9+
· ╰── Use `<Image />` from `next/image` instead.
910
7 │ src="/test.png"
11+
· ─────┬─────
12+
· ╰── Use a static image import instead.
13+
8 │ alt="Test picture"
1014
╰────
11-
help: See https://nextjs.org/docs/messages/no-img-element
15+
help: Consider using `<Image />` from `next/image` or a custom image loader to automatically optimize images.
16+
See https://nextjs.org/docs/messages/no-img-element
1217
13-
eslint-plugin-next(no-img-element): Prevent usage of `<img>` element due to slower LCP and higher bandwidth.
18+
⚠ eslint-plugin-next(no-img-element): Using `<img>` could result in slower LCP and higher bandwidth.
1419
╭─[no_img_element.tsx:5:17]
1520
4 │ return (
1621
5 │ <img
17-
· ───
22+
· ─┬─
23+
· ╰── Use `<Image />` from `next/image` instead.
1824
6 │ src="/test.png"
25+
· ─────┬─────
26+
· ╰── Use a static image import instead.
27+
7 │ alt="Test picture"
1928
╰────
20-
help: See https://nextjs.org/docs/messages/no-img-element
29+
help: Consider using `<Image />` from `next/image` or a custom image loader to automatically optimize images.
30+
See https://nextjs.org/docs/messages/no-img-element
31+
32+
⚠ eslint-plugin-next(no-img-element): Using `<img>` could result in slower LCP and higher bandwidth.
33+
╭─[no_img_element.tsx:4:7]
34+
3 │ export const MyComponent = () => (
35+
4 │ <img src={somePicture.src} alt='foo' />
36+
· ─┬─
37+
· ╰── Use `<Image />` from `next/image` instead.
38+
5 │ );
39+
╰────
40+
help: Consider using `<Image />` from `next/image` or a custom image loader to automatically optimize images.
41+
See https://nextjs.org/docs/messages/no-img-element

0 commit comments

Comments
 (0)