Skip to content

Commit

Permalink
Create rule S6788: Disallow usage of findDOMNode (`react/no-find-dom-…
Browse files Browse the repository at this point in the history
…node`) (#4359)
  • Loading branch information
vdiez authored Nov 6, 2023
1 parent fdc7336 commit 037559d
Show file tree
Hide file tree
Showing 12 changed files with 272 additions and 0 deletions.
20 changes: 20 additions & 0 deletions its/ruling/src/test/expected/jsts/console/typescript-S6788.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"console:src/components/Help/Alert.tsx": [
28
],
"console:src/components/Help/Help.tsx": [
28
],
"console:src/components/PopupWrapper/PopupWrapper.tsx": [
56
],
"console:src/views/ProjectRootView/AddModelPopup.tsx": [
163
],
"console:src/views/ProjectRootView/EditModelPopup.tsx": [
186
],
"console:src/views/ProjectSettingsView/ProjectSettingsView.tsx": [
216
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"desktop:app/src/ui/lib/list/list.tsx": [
1016,
1240
]
}
19 changes: 19 additions & 0 deletions its/ruling/src/test/expected/jsts/sonar-web/javascript-S6788.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"sonar-web:src/main/js/components/mixins/resize-mixin.js": [
19
],
"sonar-web:src/main/js/components/mixins/tooltips-mixin.js": [
16
],
"sonar-web:src/main/js/main/nav/component/component-nav.js": [
37
],
"sonar-web:tests/apps/background-tasks-test.js": [
68
],
"sonar-web:tests/apps/system-test.js": [
15,
73,
89
]
}
34 changes: 34 additions & 0 deletions packages/jsts/src/rules/S6788/decorator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* 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.
*/
// https://sonarsource.github.io/rspec/#/rspec/S6788/javascript

import { Rule } from 'eslint';
import { interceptReport } from '../helpers';

// Rewording issue message reported by the core implementation.
export function decorate(rule: Rule.RuleModule): Rule.RuleModule {
rule.meta!.messages!['noFindDOMNode'] =
"Do not use findDOMNode. It doesn't work with function components and is deprecated in StrictMode.";
return interceptReport(rule, (context, reportDescriptor) => {
context.report({
...reportDescriptor,
});
});
}
23 changes: 23 additions & 0 deletions packages/jsts/src/rules/S6788/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* 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.
*/
import { rules } from 'eslint-plugin-react';
import { decorate } from './decorator';

export const rule = decorate(rules['no-find-dom-node']);
51 changes: 51 additions & 0 deletions packages/jsts/src/rules/S6788/unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* 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.
*/
import { rule } from './';
import { TypeScriptRuleTester } from '../tools';

const ruleTester = new TypeScriptRuleTester();

ruleTester.run(`Decorated rule should reword the issue message`, rule, {
valid: [
{
code: `type Foo = () => number;`,
},
],
invalid: [
{
code: `
class MyComponent extends Component {
componentDidMount() {
findDOMNode(this).scrollIntoView();
}
render() {
return <div />
}
}
`,
errors: [
{
message:
"Do not use findDOMNode. It doesn't work with function components and is deprecated in StrictMode.",
},
],
},
],
});
2 changes: 2 additions & 0 deletions packages/jsts/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ import { rule as S3415 } from './S3415'; // inverted-assertion-arguments
import { rule as S6477 } from './S6477'; // jsx-key
import { rule as S6481 } from './S6481'; // jsx-no-constructed-context-values
import { rule as S6749 } from './S6749'; // jsx-no-useless-fragment
import { rule as S6788 } from './S6788'; // no-find-dom-node
import { rule as S1439 } from './S1439'; // label-position
import { rule as S5148 } from './S5148'; // link-with-target-blank
import { rule as S4622 } from './S4622'; // max-union-size
Expand Down Expand Up @@ -388,6 +389,7 @@ rules['inverted-assertion-arguments'] = S3415;
rules['jsx-key'] = S6477;
rules['jsx-no-constructed-context-values'] = S6481;
rules['jsx-no-useless-fragment'] = S6749;
rules['no-find-dom-node'] = S6788;
rules['label-position'] = S1439;
rules['link-with-target-blank'] = S5148;
rules['max-union-size'] = S4622;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ public static List<Class<? extends JavaScriptCheck>> getAllChecks() {
NoExtendNativeCheck.class,
NoExtraBindCheck.class,
NoExtraBooleanCastCheck.class,
NoFindDomNodeCheck.class,
NoForInArrayCheck.class,
NoHardcodedIpCheck.class,
NoHookSetterInBodyCheck.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

@TypeScriptRule
@JavaScriptRule
@Rule(key = "S6788")
public class NoFindDomNodeCheck implements EslintBasedCheck {

@Override
public String eslintKey() {
return "no-find-dom-node";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<h2>Why is this an issue?</h2>
<p>In React, <code>findDOMNode</code> is used to get the browser DOM node given a component instance. However, using <code>findDOMNode</code> is
fragile because the connection between the JSX node and the code manipulating the corresponding DOM node is not explicit. For example, changing the
external structure of returned JSX will affect the return value of <code>findDOMNode</code>. There are also other <a
href="https://react.dev/reference/react-dom/findDOMNode#caveats">caveats</a> when using <code>findDOMNode</code>.</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
import { Component } from 'react';
import { findDOMNode } from 'react-dom';

class AutoselectingInput extends Component {
componentDidMount() {
const input = findDOMNode(this); // Noncompliant: findDOMNode is deprecated
input.select();
}

render() {
return &lt;input defaultValue="Hello" /&gt;
}
}
</pre>
<p>Instead, one should get the component’s own DOM node from a ref. Pass your ref as the <code>ref</code> attribute to the JSX tag for which you want
to get the DOM node. This tells React to put this <code>&lt;input&gt;</code>’s DOM node into <code>inputRef.current</code>.</p>
<p>Use <code>createRef</code> to manage a specific DOM node. In modern React without class components, the equivalent code would call
<code>useRef</code> instead.</p>
<pre data-diff-id="1" data-diff-type="compliant">
import { createRef, Component } from 'react';

class AutoselectingInput extends Component {
inputRef = createRef(null);

componentDidMount() {
const input = this.inputRef.current; // Always points to the input element
input.select();
}

render() {
return (
&lt;input ref={this.inputRef} defaultValue="Hello" /&gt;
);
}
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> React Documentation - <a href="https://react.dev/reference/react-dom/findDOMNode"><code>findDOMNode</code></a> </li>
<li> React Documentation - <a href="https://react.dev/reference/react/createRef"><code>createRef</code></a> </li>
<li> React Documentation - <a href="https://react.dev/reference/react/useRef"><code>useRef</code></a> </li>
<li> React Documentation - <a href="https://react.dev/learn/manipulating-the-dom-with-refs">Manipulating the DOM with Refs</a> </li>
</ul>

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

0 comments on commit 037559d

Please sign in to comment.