Skip to content
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

fix(linter/react_perf): allow new objects, array, fns, etc in top scope #4395

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions crates/oxc_linter/src/rules/react_perf/jsx_no_jsx_as_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ declare_oxc_lint!(

impl Rule for JsxNoJsxAsProp {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if node.scope_id() == ctx.scopes().root_scope_id() {
return;
}
if let AstKind::JSXElement(jsx_elem) = node.kind() {
check_jsx_element(jsx_elem, ctx);
}
Expand Down Expand Up @@ -81,14 +84,21 @@ fn check_expression(expr: &Expression) -> Option<Span> {
fn test() {
use crate::tester::Tester;

let pass = vec![r"<Item callback={this.props.jsx} />"];

let fail = vec![
let pass = vec![
r"<Item callback={this.props.jsx} />",
r"const Foo = () => <Item callback={this.props.jsx} />",
r"<Item jsx={<SubItem />} />",
r"<Item jsx={this.props.jsx || <SubItem />} />",
r"<Item jsx={this.props.jsx ? this.props.jsx : <SubItem />} />",
r"<Item jsx={this.props.jsx || (this.props.component ? this.props.component : <SubItem />)} />",
];

let fail = vec![
r"const Foo = () => (<Item jsx={<SubItem />} />)",
r"const Foo = () => (<Item jsx={this.props.jsx || <SubItem />} />)",
r"const Foo = () => (<Item jsx={this.props.jsx ? this.props.jsx : <SubItem />} />)",
r"const Foo = () => (<Item jsx={this.props.jsx || (this.props.component ? this.props.component : <SubItem />)} />)",
];

Tester::new(JsxNoJsxAsProp::NAME, pass, fail).with_react_perf_plugin(true).test_and_snapshot();
}
18 changes: 15 additions & 3 deletions crates/oxc_linter/src/rules/react_perf/jsx_no_new_array_as_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ declare_oxc_lint!(

impl Rule for JsxNoNewArrayAsProp {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if node.scope_id() == ctx.scopes().root_scope_id() {
return;
}
if let AstKind::JSXElement(jsx_elem) = node.kind() {
check_jsx_element(jsx_elem, ctx);
}
Expand Down Expand Up @@ -103,9 +106,9 @@ fn check_expression(expr: &Expression) -> Option<Span> {
fn test() {
use crate::tester::Tester;

let pass = vec![r"<Item list={this.props.list} />"];

let fail = vec![
let pass = vec![
r"<Item list={this.props.list} />",
r"const Foo = () => <Item list={this.props.list} />",
r"<Item list={[]} />",
r"<Item list={new Array()} />",
r"<Item list={Array()} />",
Expand All @@ -114,6 +117,15 @@ fn test() {
r"<Item list={this.props.list || (this.props.arr ? this.props.arr : [])} />",
];

let fail = vec![
r"const Foo = () => (<Item list={[]} />)",
r"const Foo = () => (<Item list={new Array()} />)",
r"const Foo = () => (<Item list={Array()} />)",
r"const Foo = () => (<Item list={this.props.list || []} />)",
r"const Foo = () => (<Item list={this.props.list ? this.props.list : []} />)",
r"const Foo = () => (<Item list={this.props.list || (this.props.arr ? this.props.arr : [])} />)",
];

Tester::new(JsxNoNewArrayAsProp::NAME, pass, fail)
.with_react_perf_plugin(true)
.test_and_snapshot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ declare_oxc_lint!(

impl Rule for JsxNoNewFunctionAsProp {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if node.scope_id() == ctx.scopes().root_scope_id() {
return;
}
if let AstKind::JSXElement(jsx_elem) = node.kind() {
check_jsx_element(jsx_elem, ctx);
}
Expand Down Expand Up @@ -106,22 +109,19 @@ fn test() {
use crate::tester::Tester;

let pass = vec![
r"<Item callback={this.props.callback} />",
r"<Item promise={new Promise()} />",
r"<Item onClick={bind(foo)} />",
r"<Item prop={0} />",
r"var a;<Item prop={a} />",
r"var a;a = 1;<Item prop={a} />",
r"var a;<Item prop={a} />",
r"const Foo = () => <Item callback={this.props.callback} />",
r"const Foo = () => (<Item promise={new Promise()} />)",
r"const Foo = () => (<Item onClick={bind(foo)} />)",
r"const Foo = () => (<Item prop={0} />)",
r"const Foo = () => { var a; return <Item prop={a} /> }",
r"const Foo = () => { var a;a = 1; return <Item prop={a} /> }",
r"const Foo = () => { var a;<Item prop={a} /> }",
r"function foo ({prop1 = function(){}, prop2}) {
return <Comp prop={prop2} />
}",
r"function foo ({prop1, prop2 = function(){}}) {
return <Comp prop={prop1} />
}",
];

let fail = vec![
r"<Item prop={function(){return true}} />",
r"<Item prop={() => true} />",
r"<Item prop={new Function('a', 'alert(a)')}/>",
Expand All @@ -133,6 +133,18 @@ fn test() {
r"<Item prop={this.props.callback || (this.props.cb ? this.props.cb : function(){})} />",
];

let fail = vec![
r"const Foo = () => (<Item prop={function(){return true}} />)",
r"const Foo = () => (<Item prop={() => true} />)",
r"const Foo = () => (<Item prop={new Function('a', 'alert(a)')}/>)",
r"const Foo = () => (<Item prop={Function()}/>)",
r"const Foo = () => (<Item onClick={this.clickHandler.bind(this)} />)",
r"const Foo = () => (<Item callback={this.props.callback || function() {}} />)",
r"const Foo = () => (<Item callback={this.props.callback ? this.props.callback : function() {}} />)",
r"const Foo = () => (<Item prop={this.props.callback || this.props.callback ? this.props.callback : function(){}} />)",
r"const Foo = () => (<Item prop={this.props.callback || (this.props.cb ? this.props.cb : function(){})} />)",
];

Tester::new(JsxNoNewFunctionAsProp::NAME, pass, fail)
.with_react_perf_plugin(true)
.test_and_snapshot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ declare_oxc_lint!(

impl Rule for JsxNoNewObjectAsProp {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if node.scope_id() == ctx.scopes().root_scope_id() {
return;
}
if let AstKind::JSXElement(jsx_elem) = node.kind() {
check_jsx_element(jsx_elem, ctx);
}
Expand Down Expand Up @@ -102,16 +105,20 @@ fn check_expression(expr: &Expression) -> Option<Span> {
fn test() {
use crate::tester::Tester;

let pass = vec![r"<Item config={staticConfig} />"];
let pass = vec![
r"<Item config={staticConfig} />",
r"<Item config={{}} />",
r"const Foo = () => <Item config={staticConfig} />",
];

let fail = vec![
r"<Item config={{}} />",
r"<Item config={new Object()} />",
r"<Item config={Object()} />",
r"<div style={{display: 'none'}} />",
r"<Item config={this.props.config || {}} />",
r"<Item config={this.props.config ? this.props.config : {}} />",
r"<Item config={this.props.config || (this.props.default ? this.props.default : {})} />",
r"const Foo = () => <Item config={{}} />",
r"const Foo = () => (<Item config={new Object()} />)",
r"const Foo = () => (<Item config={Object()} />)",
r"const Foo = () => (<div style={{display: 'none'}} />)",
r"const Foo = () => (<Item config={this.props.config || {}} />)",
r"const Foo = () => (<Item config={this.props.config ? this.props.config : {}} />)",
r"const Foo = () => (<Item config={this.props.config || (this.props.default ? this.props.default : {})} />)",
];

Tester::new(JsxNoNewObjectAsProp::NAME, pass, fail)
Expand Down
24 changes: 12 additions & 12 deletions crates/oxc_linter/src/snapshots/jsx_no_jsx_as_prop.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,29 @@
source: crates/oxc_linter/src/tester.rs
---
⚠ eslint-plugin-react-perf(jsx-no-jsx-as-prop): JSX attribute values should not contain other JSX.
╭─[jsx_no_jsx_as_prop.tsx:1:12]
1 │ <Item jsx={<SubItem />} />
· ───────────
╭─[jsx_no_jsx_as_prop.tsx:1:31]
1 │ const Foo = () => (<Item jsx={<SubItem />} />)
· ───────────
╰────
help: simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).

⚠ eslint-plugin-react-perf(jsx-no-jsx-as-prop): JSX attribute values should not contain other JSX.
╭─[jsx_no_jsx_as_prop.tsx:1:30]
1 │ <Item jsx={this.props.jsx || <SubItem />} />
· ───────────
╭─[jsx_no_jsx_as_prop.tsx:1:49]
1 │ const Foo = () => (<Item jsx={this.props.jsx || <SubItem />} />)
· ───────────
╰────
help: simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).

⚠ eslint-plugin-react-perf(jsx-no-jsx-as-prop): JSX attribute values should not contain other JSX.
╭─[jsx_no_jsx_as_prop.tsx:1:46]
1 │ <Item jsx={this.props.jsx ? this.props.jsx : <SubItem />} />
· ───────────
╭─[jsx_no_jsx_as_prop.tsx:1:65]
1 │ const Foo = () => (<Item jsx={this.props.jsx ? this.props.jsx : <SubItem />} />)
· ───────────
╰────
help: simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).

⚠ eslint-plugin-react-perf(jsx-no-jsx-as-prop): JSX attribute values should not contain other JSX.
╭─[jsx_no_jsx_as_prop.tsx:1:77]
1 │ <Item jsx={this.props.jsx || (this.props.component ? this.props.component : <SubItem />)} />
· ───────────
╭─[jsx_no_jsx_as_prop.tsx:1:96]
1 │ const Foo = () => (<Item jsx={this.props.jsx || (this.props.component ? this.props.component : <SubItem />)} />)
· ───────────
╰────
help: simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).
36 changes: 18 additions & 18 deletions crates/oxc_linter/src/snapshots/jsx_no_new_array_as_prop.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,43 +2,43 @@
source: crates/oxc_linter/src/tester.rs
---
⚠ eslint-plugin-react-perf(jsx-no-new-array-as-prop): JSX attribute values should not contain Arrays created in the same scope.
╭─[jsx_no_new_array_as_prop.tsx:1:13]
1 │ <Item list={[]} />
· ──
╭─[jsx_no_new_array_as_prop.tsx:1:32]
1 │ const Foo = () => (<Item list={[]} />)
· ──
╰────
help: simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).

⚠ eslint-plugin-react-perf(jsx-no-new-array-as-prop): JSX attribute values should not contain Arrays created in the same scope.
╭─[jsx_no_new_array_as_prop.tsx:1:13]
1 │ <Item list={new Array()} />
· ───────────
╭─[jsx_no_new_array_as_prop.tsx:1:32]
1 │ const Foo = () => (<Item list={new Array()} />)
· ───────────
╰────
help: simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).

⚠ eslint-plugin-react-perf(jsx-no-new-array-as-prop): JSX attribute values should not contain Arrays created in the same scope.
╭─[jsx_no_new_array_as_prop.tsx:1:13]
1 │ <Item list={Array()} />
· ───────
╭─[jsx_no_new_array_as_prop.tsx:1:32]
1 │ const Foo = () => (<Item list={Array()} />)
· ───────
╰────
help: simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).

⚠ eslint-plugin-react-perf(jsx-no-new-array-as-prop): JSX attribute values should not contain Arrays created in the same scope.
╭─[jsx_no_new_array_as_prop.tsx:1:32]
1 │ <Item list={this.props.list || []} />
· ──
╭─[jsx_no_new_array_as_prop.tsx:1:51]
1 │ const Foo = () => (<Item list={this.props.list || []} />)
· ──
╰────
help: simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).

⚠ eslint-plugin-react-perf(jsx-no-new-array-as-prop): JSX attribute values should not contain Arrays created in the same scope.
╭─[jsx_no_new_array_as_prop.tsx:1:49]
1 │ <Item list={this.props.list ? this.props.list : []} />
· ──
╭─[jsx_no_new_array_as_prop.tsx:1:68]
1 │ const Foo = () => (<Item list={this.props.list ? this.props.list : []} />)
· ──
╰────
help: simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).

⚠ eslint-plugin-react-perf(jsx-no-new-array-as-prop): JSX attribute values should not contain Arrays created in the same scope.
╭─[jsx_no_new_array_as_prop.tsx:1:67]
1 │ <Item list={this.props.list || (this.props.arr ? this.props.arr : [])} />
· ──
╭─[jsx_no_new_array_as_prop.tsx:1:86]
1 │ const Foo = () => (<Item list={this.props.list || (this.props.arr ? this.props.arr : [])} />)
· ──
╰────
help: simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).
Loading