Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unknown expression type ThisExpression #41

Closed
ljharb opened this issue Apr 20, 2016 · 16 comments
Closed

Unknown expression type ThisExpression #41

ljharb opened this issue Apr 20, 2016 · 16 comments
Assignees

Comments

@ljharb
Copy link
Member

ljharb commented Apr 20, 2016

on v1.0.1:

TYPES[expression.type] is not a function
TypeError: TYPES[expression.type] is not a function
    at extract ($PWD/node_modules/eslint-plugin-jsx-a11y/lib/util/values/expressions/index.js:102:32)
    at Object.extractValueFromMemberExpression [as MemberExpression] ($PWD/node_modules/eslint-plugin-jsx-a11y/lib/util/values/expressions/MemberExpression.js:23:30)
    at extract ($PWD/node_modules/eslint-plugin-jsx-a11y/lib/util/values/expressions/index.js:102:32)
    at Object.extractValueFromMemberExpression [as MemberExpression] ($PWD/node_modules/eslint-plugin-jsx-a11y/lib/util/values/expressions/MemberExpression.js:23:30)
    at Object.extract [as JSXExpressionContainer] ($PWD/node_modules/eslint-plugin-jsx-a11y/lib/util/values/expressions/index.js:102:32)
    at getValue ($PWD/node_modules/eslint-plugin-jsx-a11y/lib/util/values/index.js:52:27)
    at extractValue ($PWD/node_modules/eslint-plugin-jsx-a11y/lib/util/getAttributeValue.js:24:12)
    at getAttributeValue ($PWD/node_modules/eslint-plugin-jsx-a11y/lib/util/getAttributeValue.js:41:10)
    at EventEmitter.JSXOpeningElement ($PWD/node_modules/eslint-plugin-jsx-a11y/lib/rules/img-has-alt.js:56:54)
    at emitOne (events.js:82:20)
    at EventEmitter.emit (events.js:169:7)

I added a console.log to line 101 in index.js and got:

Node {
  type: 'ThisExpression',
  start: 1419,
  end: 1423,
  loc: 
   SourceLocation {
     start: Position { line: 54, column: 17 },
     end: Position { line: 54, column: 21 } },
  range: [ 1419, 1423 ] }

Perhaps the comment "This will map correctly for *all* possible expression types." above the function is inaccurate, and it should be explicitly handling unknown expression types?

@beefancohen
Copy link
Contributor

Yeah, it should be more fault tolerant than that 😦

Mind sharing the JSX that was used so I can add a handler for a ThisExpression

@ljharb
Copy link
Member Author

ljharb commented Apr 20, 2016

I have no idea - I ran it on our huge codebase and it doesn't tell me which file it's looking at :-/

@beefancohen
Copy link
Contributor

beefancohen commented Apr 20, 2016

Ah, that's okay, I've created a function to handle ThisExpression. I'm actually not sure if the code should be more defensive because it could return false positives. In your experience would it be better for the code to fail so user can report a bug or fail gracefully even if it incorrectly warns?

@ljharb
Copy link
Member Author

ljharb commented Apr 20, 2016

I'd say it'd be better for it to fail with a meaningful error message.

@beefancohen
Copy link
Contributor

as in an actual throw new Error(``Encountered an unknown type '${type}' while trying to get the prop ${prop}'s value. Please file an issue on Github to get this resolved immediately.``) ??

@ljharb
Copy link
Member Author

ljharb commented Apr 20, 2016

sure, perhaps with the filename and line number

@ljharb
Copy link
Member Author

ljharb commented Apr 20, 2016

@evcohen not to put too much pressure on you, but this is blocking release of v8 of eslint-config-airbnb ;-) is it something you hope to get out soon?

@beefancohen
Copy link
Contributor

ah working on it now, sorry about that. will let you know when im about to publish.

@beefancohen
Copy link
Contributor

actually, let me just get a fix out for ThisExpression and then add error handling for next release. might take some time to figure out how to get propagate certain values to that function to give a meaningful error message.

@beefancohen
Copy link
Contributor

Just published v1.0.2 let me know if you face any other issues. I'm going to work on better error handling right now for every rule.

@ljharb
Copy link
Member Author

ljharb commented Apr 20, 2016

Thanks!

@ivarni
Copy link

ivarni commented Apr 28, 2016

@evcohen I just got this error with version 1.0.3 for the rule "jsx-a11y/aria-proptypes"

In our code base I found this:

<input
    aria-invalid={error ? 'true' : 'false'}
>

Removing the aria-invalid property made the error go away, so if you want to work on error-handling that will at least give you something that throws.

@beefancohen
Copy link
Contributor

thanks @ivarni will get you a fix for that right now and work on error handling today.

@beefancohen
Copy link
Contributor

Fixed ConditionalExpression handler in v1.0.4

@DanCech
Copy link

DanCech commented May 5, 2016

I'm still seeing this error with v1.0.4, the line of source that's triggering it is:

<div className="download__logo float-left"><img src={'/api/plugins/' + plugin.slug + '/logos/small'} alt={plugin.name + ' Logo'} /></div>

expression is:

Node {
  type: 'BinaryExpression',
  start: 1047,
  end: 1068,
  loc:
   SourceLocation {
     start: Position { line: 25, column: 116 },
     end: Position { line: 25, column: 137 } },
  left:
   Node {
     type: 'MemberExpression',
     start: 1047,
     end: 1058,
     loc: SourceLocation { start: [Object], end: [Object] },
     object:
      Node {
        type: 'Identifier',
        start: 1047,
        end: 1053,
        loc: [Object],
        name: 'plugin',
        range: [Object],
        _babelType: 'Identifier' },
     property:
      Node {
        type: 'Identifier',
        start: 1054,
        end: 1058,
        loc: [Object],
        name: 'name',
        range: [Object],
        _babelType: 'Identifier' },
     computed: false,
     range: [ 1047, 1058 ],
     _babelType: 'MemberExpression' },
  operator: '+',
  right:
   Node {
     type: 'Literal',
     start: 1061,
     end: 1068,
     loc: SourceLocation { start: [Object], end: [Object] },
     extra: { rawValue: ' Logo', raw: '\' Logo\'' },
     value: ' Logo',
     range: [ 1061, 1068 ],
     _babelType: 'StringLiteral',
     raw: '\' Logo\'' },
  range: [ 1047, 1068 ],
  _babelType: 'BinaryExpression' }

And sure enough, BinaryExpression isn't a key in TYPES.

Changing that line to use a template literal "fixes" the problem:

<div className="download__logo float-left"><img src={'/api/plugins/' + plugin.slug + '/logos/small'} alt={`${plugin.name} Logo`} /></div>

@beefancohen
Copy link
Contributor

@DanCech published v1.1.0 with BinaryExpression handler and basic error handling when getting an unknown expression type. Thanks for reporting 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants