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

disable-next-line doesn't work with react rules in JSX #2369

Closed
felixfbecker opened this issue Aug 4, 2019 · 14 comments
Closed

disable-next-line doesn't work with react rules in JSX #2369

felixfbecker opened this issue Aug 4, 2019 · 14 comments

Comments

@felixfbecker
Copy link

felixfbecker commented Aug 4, 2019

When using // eslint-disable-next-line react/jsx-no-bind inside JSX, the violation is not ignored as expected.

Ignoring a built-in rule, e.g. arrow-parens, works as expected (demo). I am only seeing this with react/ rules, like react/jsx-no-bind or react/forbid-dom-props.

To reproduce, lint the following code snippet with

import React from 'react'
const Comp1 = () => null
const Comp2 = () => (
    <Comp1
        key={1}
        // eslint-disable-next-line react/jsx-no-bind
        onChange={isVisible => onChangeVisibility(isVisible, i)}
    ></Comp1>
)

If you move the prop up to be the first prop, it works:

import React from 'react'
const Comp1 = () => null
const Comp2 = () => (
    <Comp1
        // eslint-disable-next-line react/jsx-no-bind
        onChange={isVisible => onChangeVisibility(isVisible, i)}
        key={1}
    ></Comp1>
)

Any other rule works too:

import React from 'react'
const Comp1 = () => null
const Comp2 = () => (
    <Comp1
        key={1}
        // eslint-disable-next-line arrow-parens
        onChange={isVisible => onChangeVisibility(isVisible, i)}
    ></Comp1>
)

eslint@5.16.0
eslint-plugin-react@7.14.3

@felixfbecker felixfbecker changed the title disable-next-line react/jsx-no-bind doesn't work disable-next-line doesn't work with react rules in JSX Aug 4, 2019
@ljharb
Copy link
Member

ljharb commented Aug 5, 2019

This seems like more of an issue for eslint itself, not for this plugin - how eslint parses JSX, and override commands, is entirely within eslint's purview.

@felixfbecker
Copy link
Author

How come it only happens with react/ rules and not built-in rules?

@ljharb
Copy link
Member

ljharb commented Aug 5, 2019

That's a fair question; it might depend on the rule and what precisely it's warning on. Can you provide an example of core rule jsx overrides that work how you expect?

@felixfbecker
Copy link
Author

Yeah, arrow-parens for example. See the issue description for a playground link

@ljharb
Copy link
Member

ljharb commented Aug 5, 2019

ah thanks, sorry i missed that.

It might be some difference in how arrow-parens reports a warning location, versus how eslint-plugin-react does. It seems worth looking into.

@golopot
Copy link
Contributor

golopot commented Aug 19, 2019

I cannot reproduce this bug.

@ljharb
Copy link
Member

ljharb commented Aug 20, 2019

@felixfbecker can you provide npm ls | grep eslint output?

@felixfbecker
Copy link
Author

Here ya go:

> yarn list | grep eslint
├─ @sourcegraph/eslint-config@0.2.0
│  ├─ @typescript-eslint/eslint-plugin@^1.13.0
│  ├─ @typescript-eslint/parser@^1.13.0
│  ├─ eslint-config-prettier@^6.0.0
│  ├─ eslint-plugin-ban@^1.2.0
│  ├─ eslint-plugin-etc@^0.0.1-beta.6
│  ├─ eslint-plugin-import@^2.18.2
│  ├─ eslint-plugin-jsdoc@^15.8.0
│  ├─ eslint-plugin-react-hooks@^1.6.1
│  └─ eslint-plugin-react@^7.14.3
├─ @types/eslint-visitor-keys@1.0.0
├─ @typescript-eslint/eslint-plugin@1.13.0
│  ├─ @typescript-eslint/experimental-utils@1.13.0
│  ├─ eslint-utils@^1.3.1
├─ @typescript-eslint/experimental-utils@1.13.0
│  ├─ @typescript-eslint/typescript-estree@1.13.0
│  └─ eslint-scope@^4.0.0
├─ @typescript-eslint/parser@1.13.0
│  ├─ @types/eslint-visitor-keys@^1.0.0
│  ├─ @typescript-eslint/experimental-utils@1.13.0
│  ├─ @typescript-eslint/typescript-estree@1.13.0
│  └─ eslint-visitor-keys@^1.0.0
├─ @typescript-eslint/typescript-estree@1.13.0
│  ├─ eslint-plugin-no-unsafe-innerhtml@1.0.16
│  ├─ eslint-visitor-keys@1.0.0
│  ├─ eslint-visitor-keys@1.0.0
│  ├─ eslint@5.15.3
│  ├─ eslint@5.15.3
│  │  ├─ eslint-scope@^4.0.3
│  │  ├─ eslint-utils@^1.3.1
│  │  ├─ eslint-visitor-keys@^1.0.0
│  │  ├─ eslint-visitor-keys@1.1.0
├─ eslint-config-prettier@6.0.0
├─ eslint-etc@0.0.1
├─ eslint-import-resolver-node@0.3.2
├─ eslint-module-utils@2.4.1
├─ eslint-plugin-ban@1.2.0
├─ eslint-plugin-etc@0.0.1-beta.6
│  ├─ eslint-etc@^0.0.1
├─ eslint-plugin-import@2.18.2
│  ├─ eslint-import-resolver-node@^0.3.2
│  ├─ eslint-module-utils@^2.4.0
├─ eslint-plugin-jsdoc@15.8.0
├─ eslint-plugin-no-unsafe-innerhtml@1.0.16
│  ├─ eslint@^3.7.1
│  ├─ eslint@3.19.0
├─ eslint-plugin-react-hooks@1.6.1
├─ eslint-plugin-react@7.14.3
├─ eslint-scope@4.0.3
├─ eslint-utils@1.4.2
│  └─ eslint-visitor-keys@^1.0.0
├─ eslint-visitor-keys@1.1.0
├─ eslint@6.3.0
│  ├─ eslint-scope@^5.0.0
│  ├─ eslint-scope@5.0.0
│  ├─ eslint-utils@^1.4.2
│  ├─ eslint-visitor-keys@^1.1.0
│  │  └─ eslint-visitor-keys@^1.1.0
│  └─ eslint-visitor-keys@^1.0.0
│  ├─ eslint-scope@^4.0.3

Failure seen just now:

image

@cmrigney
Copy link

I'm having the same issue.

image

image

@ljharb
Copy link
Member

ljharb commented Oct 11, 2019

@cmrigney in your case, you're ensuring that renderRow will be !== on successive renders, breaking any PureComponent-like optimizations in RenderOption.

@nazariydolfin
Copy link

I have got the same issue, too. /* eslint-disable */ for multi-line does not work either.

@gitjul
Copy link

gitjul commented Jun 18, 2020

this is a bit stale, but since it's not closed, and in case anyone else runs into this thread, have you tried surrounding the rule in brackets, e.g. { /* eslint-disable-next-line react/jsx-no-bind */ }?

could look like this:

<Comp1
  { /* eslint-disable-next-line react/jsx-no-bind */ }
  onChange={isVisible => onChangeVisibility(isVisible, i)}
  key={1}
></Comp1>

or this:

<Comp1
  onChange={isVisible => onChangeVisibility(isVisible, i)}   { /* eslint-disable-line react/jsx-no-bind */ }
  key={1}
></Comp1>

I just did something like that in a project after being stuck for a while and it worked.
found the solution here: eslint/eslint#7030

@ljharb ljharb closed this as completed Jun 18, 2020
@ManoharSomu

This comment has been minimized.

@ljharb

This comment has been minimized.

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

No branches or pull requests

7 participants