Extract used propTypes detection #1946

alexzherdev · 2018-08-19T06:34:59Z

Pushing for visibility.
This is failing one existing test, which will get fixed after #1939 is merged (also will need a rebase).

Resolves #963, resolves #1871.

ljharb

LGTM overall

ljharb · 2018-08-20T06:48:28Z

lib/util/usedPropTypes.js

+    'Program:exit': function() {
+      const list = components.list();
+
+      for (const component in list) {


for..in should be avoided; can this use normal array iteration methods?

Sure. Object.keys(list).forEach perhaps?
There's about a dozen other files already doing the same, handle them all?

yes, that would be great!

already done in b35ea0c 😄

alexzherdev · 2018-08-22T01:48:52Z

I went ahead and fixed other for..in occurrences; I can go back and undo those if necessary. Also rebased.
prop-types is 201 line now, no-unused-prop-types is 144 🎉

ljharb · 2018-09-19T21:33:16Z

lib/rules/boolean-prop-naming.js

@@ -248,7 +247,7 @@ module.exports = {
            }
          }

-          if (!has(list, component) || (list[component].invalidProps || []).length) {
+          if ((list[component].invalidProps || []).length) {


maybe if (list[component].invalidProps && list[component].invalidProps.length > 0)?

ljharb · 2018-09-19T21:33:46Z

lib/rules/display-name.js

-            continue;
+        Object.keys(list).forEach(component => {
+          if (list[component].hasDisplayName) {
+            return;


Object.keys(list).filter(component => !list[component].hasDisplayName).forEach?

ljharb · 2018-09-19T21:34:13Z

lib/rules/no-multi-comp.js

-        for (const component in list) {
-          if (!has(list, component) || isIgnored(list[component]) || ++i === 1) {
-            continue;
+        Object.keys(list).forEach(component => {


(component, i) and then we don't need to ++ it, nor have let i = 0 above.

Can't do this because we only want to ++i if isIgnored is false and doesn't short-circuit. I.e. we want to start reporting from the second non-ignored component.

Actually found a way to achieve this, also using filter.

ljharb · 2018-09-19T21:34:34Z

lib/rules/no-set-state.js

-        for (const component in list) {
-          if (!has(list, component) || isValid(list[component])) {
-            continue;
+        Object.keys(list).forEach(component => {


Object.keys(list).filter(component => !isValid(list[component])).forEach?

ljharb · 2018-09-19T21:34:58Z

lib/rules/no-unused-prop-types.js

-        for (const component in list) {
-          if (!has(list, component) || !mustBeValidated(list[component])) {
-            continue;
+        Object.keys(list).forEach(component => {


Object.keys(list).filter(component => mustBeValidated(list[component])).forEach?

ljharb · 2018-09-19T21:43:10Z

lib/util/usedPropTypes.js

+
+        if (LIFE_CYCLE_METHODS.indexOf(name) >= 0) {
+          return true;
+        } else if (checkAsyncSafeLifeCycles && ASYNC_SAFE_LIFE_CYCLE_METHODS.indexOf(name) >= 0) {


no need for the else

ljharb · 2018-09-19T21:43:28Z

lib/util/usedPropTypes.js

+
+    if (node.kind === 'constructor') {
+      return true;
+    } else if (LIFE_CYCLE_METHODS.indexOf(nodeKeyName) >= 0) {


also here (ie, the no-else-return eslint rule)

yeah, I wonder why this isn't warning

ljharb · 2018-09-19T21:46:08Z

lib/util/usedPropTypes.js

+    switch (type) {
+      case 'direct':
+        // Ignore Object methods
+        if (Object.prototype[name]) {


name in Object.prototype would be better, so as not to trigger getters

ljharb · 2018-09-19T21:47:47Z

lib/util/usedPropTypes.js

+      // method -- mark it for used props.
+      if (isNodeALifeCycleMethod(node.parent.parent)) {
+        node.properties.forEach((property, i) => {
+          if (i === 0) {


this is just:

if (node.properties.length > 0) { markPropTypesAsUsed(node.parent); }

the forEach seems useless?

ljharb · 2018-09-19T21:48:13Z

lib/util/usedPropTypes.js

+    'Program:exit': function() {
+      const list = components.list();
+
+      Object.keys(list).forEach(component => {


Object.keys(list).filter(component => mustBeValidated(list[component])).forEach?

ThiefMaster · 2018-10-16T11:35:02Z

I believe this change caused #2017.

The old logic was:

    function isPropTypesUsage(node) {
      const isClassUsage = (
        (utils.getParentES6Component() || utils.getParentES5Component()) &&
        node.object.type === 'ThisExpression' && node.property.name === 'props'
      );
      const isStatelessFunctionUsage = node.object.name === 'props' && !isAssignmentToProp(node);
      const isNextPropsUsage = node.object.name === 'nextProps' && (inComponentWillReceiveProps() || inShouldComponentUpdate());
      return isClassUsage || isStatelessFunctionUsage || isNextPropsUsage;
}

and it got replaced with:

  function isPropTypesUsage(node) {
    const isClassUsage = (
      (utils.getParentES6Component() || utils.getParentES5Component()) &&
      ((node.object.type === 'ThisExpression' && node.property.name === 'props')
      || isPropArgumentInSetStateUpdater(node))
    );
    const isStatelessFunctionUsage = node.object.name === 'props' && !isAssignmentToProp(node);
    return isClassUsage || isStatelessFunctionUsage || inLifeCycleMethod();
  }

ljharb reviewed Aug 20, 2018

View reviewed changes

alexzherdev force-pushed the used-prop-types-refactoring branch from 35f717c to 014e9d2 Compare August 22, 2018 01:04

alexzherdev mentioned this pull request Aug 23, 2018

Spread of any object into jsx causes react/prop-types to stop working (errors not detected) #1952

Closed

alexzherdev force-pushed the used-prop-types-refactoring branch 2 times, most recently from bdd9d22 to b35ea0c Compare September 9, 2018 01:24

ljharb reviewed Sep 19, 2018

View reviewed changes

alexzherdev added 4 commits September 19, 2018 20:33

Extract used prop types detection code

Verified

This commit was signed with the committer’s verified signature.

christophstrobl Christoph Strobl

GPG key ID: 8CC1AB53391458C8

Verified
Learn about vigilant mode

dd6f130

Add test cases

deae58e

Move used prop type flag setting to new helper

3794588

Replace for..in with Array.prototype methods

cf43b2e

alexzherdev force-pushed the used-prop-types-refactoring branch from b35ea0c to 5678833 Compare September 20, 2018 04:33

Filter arrays instead of if conditions

fd03bd7

alexzherdev force-pushed the used-prop-types-refactoring branch from 5678833 to fd03bd7 Compare September 20, 2018 04:45

ljharb approved these changes Sep 20, 2018

View reviewed changes

ljharb merged commit bd083bf into jsx-eslint:master Sep 20, 2018

alexzherdev deleted the used-prop-types-refactoring branch September 20, 2018 20:06

renovate bot mentioned this pull request Dec 28, 2018

Update dependency eslint-plugin-react to v7.12.0 mozilla/delivery-console#670

Merged

1 task

renovate-bot mentioned this pull request Dec 28, 2018

Update dependency eslint-plugin-react to v7.12.4 ampproject/amp-wp#1782

Closed

1 task

ghost mentioned this pull request Jan 12, 2019

Update linters uzh-bf/klicker-uzh#859

Merged

1 task

This was referenced Aug 17, 2019

[Snyk] Upgrade eslint-plugin-react from 7.4.0 to 7.14.3 Financial-Times/ip-material-ui-theme#25

Open

[Snyk] Upgrade eslint-plugin-react from 7.5.1 to 7.14.3 Alexander1984z/chrome-extensions-manager#4

Open

This was referenced Sep 22, 2019

[Snyk] Upgrade eslint-plugin-react from 7.7.0 to 7.14.3 Matthelonianxl/standard#2

Open

[Snyk] Upgrade eslint-plugin-react from 7.4.0 to 7.14.3 Ivajkin/react-redux-action#4

Open

snyk-bot mentioned this pull request Sep 30, 2019

[Snyk] Upgrade eslint-plugin-react from 7.5.1 to 7.14.3 Matthelonianxl/nifty-wallet#2

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract used propTypes detection #1946

Extract used propTypes detection #1946

alexzherdev commented Aug 19, 2018 •

edited

Loading

ljharb left a comment

ljharb Aug 20, 2018

alexzherdev Aug 20, 2018

ljharb Sep 19, 2018

alexzherdev Sep 19, 2018

alexzherdev commented Aug 22, 2018

ljharb Sep 19, 2018

ljharb Sep 19, 2018

ljharb Sep 19, 2018

alexzherdev Sep 20, 2018

alexzherdev Sep 20, 2018

ljharb Sep 19, 2018

ljharb Sep 19, 2018

ljharb Sep 19, 2018

ljharb Sep 19, 2018

alexzherdev Sep 20, 2018

ljharb Sep 19, 2018

ljharb Sep 19, 2018

ljharb Sep 19, 2018

ThiefMaster commented Oct 16, 2018

Extract used propTypes detection #1946

Extract used propTypes detection #1946

Conversation

alexzherdev commented Aug 19, 2018 • edited Loading

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexzherdev commented Aug 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThiefMaster commented Oct 16, 2018

alexzherdev commented Aug 19, 2018 •

edited

Loading