-
-
Notifications
You must be signed in to change notification settings - Fork 738
feat(linter/plugins): add react/no-unsafe #16532
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
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,329 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use oxc_ast::AstKind; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use oxc_diagnostics::OxcDiagnostic; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use oxc_macros::declare_oxc_lint; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use oxc_span::{GetSpan, Span}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use schemars::JsonSchema; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use serde::{Deserialize, Serialize}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| AstNode, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| context::{ContextHost, LintContext}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rule::{DefaultRuleConfig, Rule}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| utils::{get_parent_component, is_es5_component}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn no_unsafe_diagnostic(method_name: &str, span: Span) -> OxcDiagnostic { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| OxcDiagnostic::warn(format!("Unsafe lifecycle method `{method_name}` is not allowed")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .with_help(format!( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "`{method_name}` is deprecated and may be removed in future React versions. Consider using alternative lifecycle methods or hooks." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .with_label(span) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[schemars(rename_all = "camelCase")] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[serde(rename_all = "camelCase", default)] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[derive(Default)] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| struct NoUnsafeConfig { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[serde(default)] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| check_aliases: bool, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[derive(Debug, Default, Clone, Deserialize, Serialize, JsonSchema)] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub struct NoUnsafe(NoUnsafeConfig); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| declare_oxc_lint!( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// ### What it does | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// This rule identifies and restricts the use of unsafe React lifecycle methods. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// ### Why is this bad? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Certain lifecycle methods (`componentWillMount`, `componentWillReceiveProps`, and `componentWillUpdate`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// are considered unsafe and have been deprecated since React 16.9. They are frequently misused and cause | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// problems in async rendering. Using their `UNSAFE_` prefixed versions or the deprecated names themselves | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// should be avoided. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+43
to
+45
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// are considered unsafe and have been deprecated since React 16.9. They are frequently misused and cause | |
| /// problems in async rendering. Using their `UNSAFE_` prefixed versions or the deprecated names themselves | |
| /// should be avoided. | |
| /// are considered unsafe. The `UNSAFE_`-prefixed versions of these methods were introduced in React 16.3, | |
| /// and this rule flags their usage starting from that version. The original (non-prefixed) versions were | |
| /// deprecated in React 16.9 and removed in React 17.0. By default, only the `UNSAFE_`-prefixed methods are | |
| /// flagged; optionally, the non-prefixed versions (aliases) are also flagged when `checkAliases` is enabled. | |
| /// These methods are frequently misused and cause problems in async rendering. Their usage should be avoided. |
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.
For performance, we have some code that does static analysis on the rules and detects what node types it looks at. It will be able to optimize this rule better if we organize it like this:
| let react_version = | |
| ctx.settings().react.version.as_ref().and_then(|v| parse_react_version(v.as_str())); | |
| if let AstKind::MethodDefinition(method_def) = node.kind() | |
| && let Some(name) = method_def.key.static_name() | |
| && is_unsafe_method(name.as_ref(), self.0.check_aliases, react_version) | |
| && get_parent_component(node, ctx).is_some() | |
| { | |
| ctx.diagnostic(no_unsafe_diagnostic(name.as_ref(), method_def.key.span())); | |
| } | |
| if let AstKind::ObjectProperty(obj_prop) = node.kind() | |
| && let Some(name) = obj_prop.key.static_name() | |
| && is_unsafe_method(name.as_ref(), self.0.check_aliases, react_version) | |
| { | |
| for ancestor in ctx.nodes().ancestors(node.id()) { | |
| if is_es5_component(ancestor) { | |
| ctx.diagnostic(no_unsafe_diagnostic(name.as_ref(), obj_prop.key.span())); | |
| break; | |
| } | |
| } | |
| } | |
| match node.kind() { | |
| AstKind::MethodDefinition(method_def) => { | |
| let react_version = ctx.settings().react.version.as_ref().and_then(|v| parse_react_version(v.as_str())); | |
| if let Some(name) = method_def.key.static_name() | |
| && is_unsafe_method(name.as_ref(), self.0.check_aliases, react_version) | |
| && get_parent_component(node, ctx).is_some() | |
| { | |
| ctx.diagnostic(no_unsafe_diagnostic(name.as_ref(), method_def.key.span())); | |
| } | |
| } | |
| AstKind::ObjectProperty(obj_prop) => { /* ... */ } | |
| _ => {} | |
| } |
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.
This could be condensed into a single match block I think:
| if check_unsafe_prefix | |
| && matches!( | |
| name, | |
| "UNSAFE_componentWillMount" | |
| | "UNSAFE_componentWillReceiveProps" | |
| | "UNSAFE_componentWillUpdate" | |
| ) | |
| { | |
| return true; | |
| } | |
| if check_aliases | |
| && matches!( | |
| name, | |
| "componentWillMount" | "componentWillReceiveProps" | "componentWillUpdate" | |
| ) | |
| { | |
| return true; | |
| } | |
| false | |
| match name { | |
| "UNSAFE_componentWillMount" | |
| | "UNSAFE_componentWillReceiveProps" | |
| | "UNSAFE_componentWillUpdate" if check_unsafe_prefix => true, | |
| "componentWillMount" | "componentWillReceiveProps" | "componentWillUpdate" if check_aliases => true, | |
| _ => false | |
| } |
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.
This version written by @taearls is slightly better since it doesn't allocate:
/// Parse version string like "16.14.0" into (major, minor, patch)
fn parse_version(version: &str) -> Option<(u32, u32, u32)> {
// Avoid Vec allocation by using split_once and split_once again
let (major_str, rest) = version.split_once('.')?;
let (minor_str, patch_str) = rest.split_once('.')?;
let major = major_str.parse::<u32>().ok()?;
let minor = minor_str.parse::<u32>().ok()?;
let patch = patch_str.parse::<u32>().ok()?;
Some((major, minor, patch))
}Once of these PRs lands, we should move this into a shared util file.
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.
The original version of this rule includes specific method recommendations, we should try including those instead of just saying "alternative lifecycle methods or hooks":
https://github.com/jsx-eslint/eslint-plugin-react/blob/f2869fd6dc76ceb863c5e2aeea8bf4d392508775/lib/rules/no-unsafe.js#L58-L69