Eslint-plugin-react: jsx-no-duplicate-props: TypeError: name.toLowerCase is not a function

[sompylasar]

I use [email protected]

The latest master as of today contains the same code that fails: https://github.com/yannickcr/eslint-plugin-react/blob/21f684a738af7a6f2b1ec9517b5040162a750789/lib/rules/jsx-no-duplicate-props.js#L48

TypeError: name.toLowerCase is not a function
    at /__CENSORED__/node_modules/eslint-plugin-react/lib/rules/jsx-no-duplicate-props.js:48:25
    at Array.forEach (native)
    at EventEmitter.JSXOpeningElement (/__CENSORED__/node_modules/eslint-plugin-react/lib/rules/jsx-no-duplicate-props.js:40:25)
    at emitOne (events.js:101:20)
    at EventEmitter.emit (events.js:188:7)
    at NodeEventGenerator.enterNode (/__CENSORED__/node_modules/eslint/lib/util/node-event-generator.js:40:22)
    at CodePathAnalyzer.enterNode (/__CENSORED__/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:608:23)
    at CommentEventGenerator.enterNode (/__CENSORED__/node_modules/eslint/lib/util/comment-event-generator.js:97:23)
    at Controller.enter (/__CENSORED__/node_modules/eslint/lib/eslint.js:898:36)
    at Controller.__execute (/__CENSORED__/node_modules/estraverse/estraverse.js:397:31)

The issue happens when I (accidentally) put a colon after a prop name, for example:

      <SomeComponent
        minValue={5000}
        maxValue={750000}
        someProp:   //<<<< The colon here triggers the error.
        value={this.state.value}
        onChange={this._handleChange}
      />

[ljharb]

That's invalid jsx syntax - I don't think it's a reasonable expectation that rules should work when your syntax is invalid.

In other words, even if you disabled that rule, eslint itself should be dying on the syntax.

[sompylasar]

Right, but it should not throw, i.e. it should just somehow ignore the input if the rule does not apply.

[ljharb]

That's a fair point; that the rule should be silent when there's a parse error. Although, that seems like something eslint itself should be handling perhaps?

[sompylasar]

Maybe. Anyway, the faulty code assumes there is a name property on the name of the decl (decl.name.name) and it is a string, but something went wrong and invalidated the assumption. If there has been a parse error, the code might not have gotten that far I think.

[marcelmokos]

I use [email protected]

Same problem here.
I have modified the code to console.log decl if name is not string.

var name = decl.name.name;
if (typeof name !== 'string') console.log({decl});
if (ignoreCase) {
  name = name.toLowerCase();
}

This is output

$ eslint "src/**" 
{ decl: 
   Node {
     type: 'JSXAttribute',
     start: 40,
     end: 82,
     loc: SourceLocation { start: [Object], end: [Object] },
     name: 
      Node {
        type: 'JSXNamespacedName',
        start: 40,
        end: 51,
        loc: [Object],
        namespace: [Object],
        name: [Object],
        range: [Object],
        _babelType: 'JSXNamespacedName' },
     value: 
      Node {
        type: 'Literal',
        start: 52,
        end: 82,
        loc: [Object],
        extra: null,
        value: 'http://www.w3.org/1999/xlink',
        range: [Object],
        _babelType: 'StringLiteral',
        raw: '"http://www.w3.org/1999/xlink"' },
     range: [ 40, 82 ],
     _babelType: 'JSXAttribute' } }
name.toLowerCase is not a function
TypeError: name.toLowerCase is not a function
    at /Users/user/work/Web-React/node_modules/eslint-plugin-react/lib/rules/jsx-no-duplicate-props.js:53:25
    at Array.forEach (native)
    at EventEmitter.JSXOpeningElement (/Users/user/work/Web-React/node_modules/eslint-plugin-react/lib/rules/jsx-no-duplicate-props.js:42:25)
    at emitOne (events.js:101:20)
    at EventEmitter.emit (events.js:191:7)
    at NodeEventGenerator.applySelector (/Users/user/work/Web-React/node_modules/eslint/lib/util/node-event-generator.js:265:26)
    at NodeEventGenerator.applySelectors (/Users/user/work/Web-React/node_modules/eslint/lib/util/node-event-generator.js:294:22)
    at NodeEventGenerator.enterNode (/Users/user/work/Web-React/node_modules/eslint/lib/util/node-event-generator.js:308:14)
    at CodePathAnalyzer.enterNode (/Users/user/work/Web-React/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:602:23)
    at CommentEventGenerator.enterNode (/Users/user/work/Web-React/node_modules/eslint/lib/util/comment-event-generator.js:98:23)
    at Traverser.enter (/Users/user/work/Web-React/node_modules/eslint/lib/eslint.js:929:36)
    at Traverser.__execute (/Users/user/work/Web-React/node_modules/estraverse/estraverse.js:397:31)
    at Traverser.traverse (/Users/user/work/Web-React/node_modules/estraverse/estraverse.js:501:28)
    at Traverser.traverse (/Users/user/work/Web-React/node_modules/eslint/lib/util/traverser.js:31:22)
    at EventEmitter.module.exports.api.verify (/Users/user/work/Web-React/node_modules/eslint/lib/eslint.js:926:23)
    at processText (/Users/user/work/Web-React/node_modules/eslint/lib/cli-engine.js:264:31)

[DianaSuvorova]

@marcelmokos, seems like your change would fix the issue. You may just need to add few test cases. Why don't you create a PR for this repo?

**I will be happy to help if you have any questions 😃

[DianaSuvorova]

Here is another test case to repro the error:

const invalidJSX = () => {
  return (
    <svg >
          <use xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="#myIcon"></use>
    </svg>
  );
};

I understand that this is invalid jsx. But the error it throws makes no sense (TypeError: name.toLowerCase is not a function).
I think the better idea for the linter in general would be just skipping invalid syntax.

[sompylasar]

Please see my comment to the commit https://github.com/marcelmokos/eslint-plugin-react/commit/6918bba271fccec28f40561f91e9808d5b91d7be
It's a workaround for this specific exception, not a fix for the root cause which is an AST node instead of a string in decl.name.name which has to be handled as a valid case.

[DianaSuvorova]

@sompylasar, could you please provide complete test case for the behaviour you are describing?

[sompylasar]

@DianaSuvorova It's not about the test case (it's the same), it's about the proposed solution. Please see this comment: https://github.com/marcelmokos/eslint-plugin-react/commit/6918bba271fccec28f40561f91e9808d5b91d7be#commitcomment-23208488

[DianaSuvorova]

@sompylasar , let's continue this discussion there

[ljharb]

Closed in #1319.