Skip to content

Commit

Permalink
Create rule S6789: Disallow usage of isMounted (react/no-is-mounted)
Browse files Browse the repository at this point in the history
  • Loading branch information
yassin-kammoun-sonarsource committed Nov 6, 2023
1 parent fdc7336 commit 6638942
Show file tree
Hide file tree
Showing 15 changed files with 158 additions and 0 deletions.
@@ -0,0 +1,5 @@
{
"file-for-rules:S6789.js": [
14
]
}
Expand Up @@ -67,5 +67,8 @@
],
"file-for-rules:S6761.js": [
3
],
"file-for-rules:S6789.js": [
7
]
}
Expand Up @@ -179,6 +179,9 @@
"file-for-rules:S6766.js": [
0
],
"file-for-rules:S6789.js": [
0
],
"file-for-rules:S6793.js": [
0
],
Expand Down
Expand Up @@ -10,5 +10,8 @@
],
"file-for-rules:S6329.js": [
1
],
"file-for-rules:S6789.js": [
1
]
}
Expand Up @@ -50,5 +50,8 @@
],
"file-for-rules:S6439.js": [
5
],
"file-for-rules:S6789.js": [
4
]
}
@@ -1,5 +1,8 @@
{
"file-for-rules:S6435.js": [
3
],
"file-for-rules:S6789.js": [
11
]
}
@@ -0,0 +1,5 @@
{
"file-for-rules:S6789.js": [
6
]
}
@@ -0,0 +1,5 @@
{
"file-for-rules:S6789.js": [
7
]
}
Expand Up @@ -301,6 +301,7 @@ static void runRulingTest(String projectKey, String sources, String exclusions,
.setProperty("sonar.lits.differences", differencesPath.toString())
.setProperty("sonar.exclusions", actualExclusions)
.setProperty("sonar.javascript.node.maxspace", "2048")
.setProperty("sonar.nodejs.executable", "/usr/local/bin/node")
.setProperty("sonar.javascript.maxFileSize", "4000")
.setProperty("sonar.cpd.exclusions", "**/*")
.setProperty("sonar.internal.analysis.failFast", "true");
Expand Down
14 changes: 14 additions & 0 deletions its/sources/jsts/custom/S6789.js
@@ -0,0 +1,14 @@
import * as React from 'react';
class MyComponent extends React.Component {
componentDidMount() {
mydatastore.subscribe(this);
}
dataHandler() {
if (this.isMounted()) { // Noncompliant: isMounted() hides the error
//...
}
}
render() {
//...
}
};
Expand Up @@ -275,6 +275,7 @@ public static List<Class<? extends JavaScriptCheck>> getAllChecks() {
NoInferrableTypesCheck.class,
NoInvalidAwaitCheck.class,
NoInvertedBooleanCheckCheck.class,
NoIsMountedCheck.class,
NoLoneBlocksCheck.class,
NoLonelyIfCheck.class,
NoLossOfPrecisionCheck.class,
Expand Down
@@ -0,0 +1,36 @@
/**
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.javascript.checks;

import org.sonar.check.Rule;
import org.sonar.plugins.javascript.api.EslintBasedCheck;
import org.sonar.plugins.javascript.api.JavaScriptRule;
import org.sonar.plugins.javascript.api.TypeScriptRule;

@JavaScriptRule
@TypeScriptRule
@Rule(key = "S6789")
public class NoIsMountedCheck implements EslintBasedCheck {

@Override
public String eslintKey() {
return "no-is-mounted";
}
}
@@ -0,0 +1,47 @@
<h2>Why is this an issue?</h2>
<p>React <code>isMounted()</code> is primarily used to avoid calling <code>setState()</code> after a component has unmounted, because calling
<code>setState()</code> after a component has unmounted will emit a warning. Checking <code>isMounted()</code> before calling <code>setState()</code>
does eliminate the warning, but it also defeats the purpose of the warning, which is raising awareness that the app is still holding a reference to
the component after the component has been unmounted.</p>
<p>When using ES6 classes, using <code>isMounted()</code> is already prohibited.</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
class MyComponent extends React.Component {
componentDidMount() {
mydatastore.subscribe(this);
}
dataHandler: function() {
if (this.isMounted()) { // Noncompliant: isMounted() hides the error
//...
}
}
render: function() {
//... calls dataHandler()
}
});
</pre>
<p>Find places where <code>setState()</code> might be called after a component has unmounted, and fix them. Such situations most commonly occur due to
callbacks, when a component is waiting for some data and gets unmounted before the data arrives. Ideally, any callbacks should be canceled in
<code>componentWillUnmount</code>, before the component unmounts.</p>
<pre data-diff-id="1" data-diff-type="compliant">
class MyComponent extends React.Component {
componentDidMount() {
mydatastore.subscribe(this);
}
dataHandler: function() {
//...
}
render() {
//...
}
componentWillUnmount() {
mydatastore.unsubscribe(this);
}
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> React Documentation - <a href="https://legacy.reactjs.org/blog/2015/12/16/ismounted-antipattern.html"><code>isMounted</code> is an
Antipattern</a> </li>
</ul>

@@ -0,0 +1,28 @@
{
"title": "React\u0027s \"isMounted\" should not be used",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"react"
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-6789",
"sqKey": "S6789",
"scope": "All",
"quickfix": "unknown",
"code": {
"impacts": {
"MAINTAINABILITY": "MEDIUM",
"RELIABILITY": "MEDIUM"
},
"attribute": "CONVENTIONAL"
},
"compatibleLanguages": [
"JAVASCRIPT",
"TYPESCRIPT"
]
}
Expand Up @@ -292,6 +292,7 @@
"S6772",
"S6774",
"S6775",
"S6789",
"S6793",
"S6807",
"S6811",
Expand Down

0 comments on commit 6638942

Please sign in to comment.