Fixes eslint warning when node type is ChainExpression (#19680)

* Add babel parser which supports ChainExpression * Add and fix tests for new babel eslint parser * extract function to mark node * refactor for compatibility with eslint v7.7.0+ * Update eslint to v7.7.0 Update hook test since eslint now supports nullish coalescing

Author: Pascal Fong Kye

package.json

@@ -7,6 +7,7 @@
 "@babel/cli": "^7.10.5",
 "@babel/code-frame": "^7.10.4",
 "@babel/core": "^7.11.1",
+"@babel/eslint-parser": "^7.11.4",
 "@babel/helper-module-imports": "^7.10.4",
 "@babel/parser": "^7.11.3",
 "@babel/plugin-external-helpers": "^7.10.4",
@@ -46,7 +47,7 @@
 "create-react-class": "^15.6.3",
 "danger": "^9.2.10",
 "error-stack-parser": "^2.0.6",
-"eslint": "^7.0.0",
+"eslint": "^7.7.0",
 "eslint-config-fbjs": "^1.1.1",
 "eslint-config-prettier": "^6.9.0",
 "eslint-plugin-babel": "^5.3.0",

packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js

@@ -7972,6 +7972,11 @@ new ESLintTester({
  parserOptions,
 }).run('react-hooks',ReactHooksESLintRule,tests);
 
+newESLintTester({
+parser: require.resolve('@babel/eslint-parser'),
+ parserOptions,
+}).run('react-hooks',ReactHooksESLintRule,tests);
+
 newESLintTester({
 parser: require.resolve('@typescript-eslint/parser'),
  parserOptions,

packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js

@@ -752,9 +752,7 @@ const tests = {
 errors: [
 conditionalError('useState'),
 conditionalError('useState'),
-// TODO: ideally this *should* warn, but ESLint
-// doesn't plan full support for ?? until it advances.
-// conditionalError('useState'),
+conditionalError('useState'),
 ],
 },
 {

packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js

@@ -814,9 +814,10 @@ export default {
 letmaybeID=declaredDependencyNode;
 while(
 maybeID.type==='MemberExpression'||
-maybeID.type==='OptionalMemberExpression'
+maybeID.type==='OptionalMemberExpression'||
+maybeID.type==='ChainExpression'
 ){
-maybeID=maybeID.object;
+maybeID=maybeID.object||maybeID.expression.object;
 }
 constisDeclaredInComponent=!componentScope.through.some(
 ref=>ref.identifier===maybeID,
@@ -1590,6 +1591,27 @@ function getDependency(node) {
 }
 }
 
+/**
+ * Mark a node as either optional or required.
+ * Note: If the node argument is an OptionalMemberExpression, it doesn't necessarily mean it is optional.
+ * It just means there is an optional member somewhere inside.
+ * This particular node might still represent a required member, so check .optional field.
+ */
+functionmarkNode(node,optionalChains,result){
+if(optionalChains){
+if(node.optional){
+// We only want to consider it optional if *all* usages were optional.
+if(!optionalChains.has(result)){
+// Mark as (maybe) optional. If there's a required usage, this will be overridden.
+optionalChains.set(result,true);
+}
+}else{
+// Mark as required.
+optionalChains.set(result,false);
+}
+}
+}
+
 /**
  * Assuming () means the passed node.
  * (foo) -> 'foo'
@@ -1609,30 +1631,20 @@ function analyzePropertyChain(node, optionalChains) {
 constobject=analyzePropertyChain(node.object,optionalChains);
 constproperty=analyzePropertyChain(node.property,null);
 constresult=`${object}.${property}`;
-if(optionalChains){
-// Mark as required.
-optionalChains.set(result,false);
-}
+markNode(node,optionalChains,result);
 returnresult;
 }elseif(node.type==='OptionalMemberExpression'&&!node.computed){
 constobject=analyzePropertyChain(node.object,optionalChains);
 constproperty=analyzePropertyChain(node.property,null);
 constresult=`${object}.${property}`;
-if(optionalChains){
-// Note: OptionalMemberExpression doesn't necessarily mean this node is optional.
-// It just means there is an optional member somewhere inside.
-// This particular node might still represent a required member, so check .optional field.
-if(node.optional){
-// We only want to consider it optional if *all* usages were optional.
-if(!optionalChains.has(result)){
-// Mark as (maybe) optional. If there's a required usage, this will be overridden.
-optionalChains.set(result,true);
-}
-}else{
-// Mark as required.
-optionalChains.set(result,false);
-}
-}
+markNode(node,optionalChains,result);
+returnresult;
+}elseif(node.type==='ChainExpression'&&!node.computed){
+constexpression=node.expression;
+constobject=analyzePropertyChain(expression.object,optionalChains);
+constproperty=analyzePropertyChain(expression.property,null);
+constresult=`${object}.${property}`;
+markNode(expression,optionalChains,result);
 returnresult;
 }else{
 thrownewError(`Unsupported node type: ${node.type}`);

scripts/eslint-rules/no-production-logging.js

@@ -9,75 +9,77 @@
 
 'use strict';
 
-module.exports=function(context){
-functionisInDEVBlock(node){
-letdone=false;
-while(!done){
-letparent=node.parent;
-if(!parent){
-returnfalse;
-}
-if(
-parent.type==='IfStatement'&&
-node===parent.consequent&&
-parent.test.type==='Identifier'&&
-// This is intentionally strict so we can
-// see blocks of DEV-only code at once.
-parent.test.name==='__DEV__'
-){
-returntrue;
+module.exports={
+meta: {
+fixable: 'code',
+},
+create: function(context){
+functionisInDEVBlock(node){
+letdone=false;
+while(!done){
+letparent=node.parent;
+if(!parent){
+returnfalse;
+}
+if(
+parent.type==='IfStatement'&&
+node===parent.consequent&&
+parent.test.type==='Identifier'&&
+// This is intentionally strict so we can
+// see blocks of DEV-only code at once.
+parent.test.name==='__DEV__'
+){
+returntrue;
+}
+node=parent;
 }
-node=parent;
 }
-}
 
-functionreportWrapInDEV(node){
-context.report({
-node: node,
-message: `Wrap console.{{identifier}}() in an "if (__DEV__) {}" check`,
-data: {
-identifier: node.property.name,
-},
-fix: function(fixer){
-return[
-fixer.insertTextBefore(node.parent,`if (__DEV__) {`),
-fixer.insertTextAfter(node.parent,'}'),
-];
-},
-});
-}
+functionreportWrapInDEV(node){
+context.report({
+node: node,
+message: `Wrap console.{{identifier}}() in an "if (__DEV__) {}" check`,
+data: {
+identifier: node.property.name,
+},
+fix: function(fixer){
+return[
+fixer.insertTextBefore(node.parent,`if (__DEV__) {`),
+fixer.insertTextAfter(node.parent,'}'),
+];
+},
+});
+}
 
-functionreportUnexpectedConsole(node){
-context.report({
-node: node,
-message: `Unexpected use of console`,
-});
-}
+functionreportUnexpectedConsole(node){
+context.report({
+node: node,
+message: `Unexpected use of console`,
+});
+}
 
-return{
-meta: {
-fixable: 'code',
-},
-MemberExpression: function(node){
-if(
-node.object.type==='Identifier'&&
-node.object.name==='console'&&
-node.property.type==='Identifier'
-){
-switch(node.property.name){
-case'error':
-case'warn': {
-if(!isInDEVBlock(node)){
-reportWrapInDEV(node);
+return{
+MemberExpression: function(node){
+if(
+node.object.type==='Identifier'&&
+node.object.name==='console'&&
+node.property.type==='Identifier'
+){
+switch(node.property.name){
+case'error':
+case'warn': {
+if(!isInDEVBlock(node)){
+reportWrapInDEV(node);
+}
+break;
+}
+default: {
+reportUnexpectedConsole(node);
+break;
 }
-break;
-}
-default: {
-reportUnexpectedConsole(node);
-break;
 }
 }
-}
-},
-};
+},
+};
+},
 };