Skip to content
This repository has been archived by the owner on Aug 18, 2021. It is now read-only.

React is defined but never used #6

Closed
gaearon opened this issue Feb 27, 2015 · 18 comments
Closed

React is defined but never used #6

gaearon opened this issue Feb 27, 2015 · 18 comments

Comments

@gaearon
Copy link
Member

gaearon commented Feb 27, 2015

Not sure if it's the same as #5.

I get

error  React is defined but never used         no-unused-vars

in a JS file that doesn't reference React directly, but uses JSX (and thus needs to have React in scope).

@glenjamin
Copy link

See also eslint/eslint#1867

I think this will need to be a custom rule

@gaearon
Copy link
Member Author

gaearon commented Feb 28, 2015

Wow, them closing that issue is a real bummer.

I see their reasoning but it's ridiculous to assume anyone would go over 500 components in a codebase to surround every React require with an eslint comment.

Of course it's a "right" decision by ESLint but it directly hurts users if there is no way to work around it using just the config.

@nzakas

@gaearon
Copy link
Member Author

gaearon commented Feb 28, 2015

(I retract my comment if it's possible with a custom rule. But a custom rule can't change the behavior of a builtin rule, can it?)

@sebmck
Copy link
Contributor

sebmck commented Feb 28, 2015

@gaearon Agreed.

@nzakas

No. ESLint supports JSX syntax as defined in the spec. We don't specifically support React, just like we don't specifically support jQuery.

You already have rules that support React usage of JSX. JSXIdentifier is explicitly special cased in the no-undef rule. There's nothing in the JSX spec that says that a JSXIdentifier refers to a local binding as the JSX spec does not dictate any logical behaviour beyond syntax.

@gaearon
Copy link
Member Author

gaearon commented Feb 28, 2015

By the way it's not that bad now (most files using JSX also use React.createClass), but it will get worse after 0.13 when components can be plain ES6 classes.

I know JSX may get decoupled from React in the future (see http://facebook.github.io/react/blog/2015/02/24/streamlining-react-elements.html, "Problem: It Couples JSX to React") but we're not there yet.

@glenjamin
Copy link

I think it should be fairly straightforward to replace the no-undef and no-unused rules with ones that understand React?

I haven't looked into it but I can see why this shouldn't live in eslint core.

The idea is we'd want some injection point to say that <Jsx /> is the use of React and Jsx variables (and that <jsx /> uses React and no other variables)

@gaearon
Copy link
Member Author

gaearon commented Feb 28, 2015

It would probably be nicer if there was a way to customize JSX parser in the config to specify the variables "required" to be in scope by using JSX. I don't know if it's possible though.

The idea is not to tie it to React, but to allow configuration to specify "React", or anything else, is required when file uses JSX. I think it's sensible in case other libraries start to use it for other purposes.

@nzakas
Copy link

nzakas commented Feb 28, 2015

As I mentioned in the ESLint issue, you can manually turn off variable checking for React:

/*eslint-disable no-unused-vars*/
var React = require('react');
/*eslint-enable no-unused-vars*/

In the next version of ESLint, you'll be able to do it with just one comment:

var React = require('react');    // eslint-disable-line no-unused-vars

The exception cases we have for JSX aren't the same category as this. We can easily tell that a JSX element represents an identifier and should be counted. This change would require us to hardcode knowledge of the React local variable, and that's what I don't want to do.

@glenjamin
Copy link

I agree that hard-coding react would be a bad idea, it would still be useful if there were some extension point to say what variables JSX uses.

If React is not set globally, then using JSX in a file without a react import causes an error.

In the current spec lowercase JSX produces a string, but beginning the tag with an uppercase character is a variable - is this currently hard-coded?

@nzakas
Copy link

nzakas commented Feb 28, 2015

The source for the rule is here, check it out yourself:
https://github.com/eslint/eslint/blob/master/lib/rules/no-unused-vars.js

The complexity here is that React is not set globally, it's being created locally (the top-level scope in browserified code is a function scope, not the global scope). You can already specify some globals that don't have to be used in the .eslintrc file. By including it specifically, you are telling ESLint, "hey, this is a local variable that I'm planning on using."

I've already shown how to disable this warning. I'm not sure why there's resistance to that. Any other type of "special configuration" would also be extra code somewhere. Why not use what's already available?

To me, the real change here is that the JSX compiler/transpiler should be inserting the React reference for you as part of that step. Forcing you to remember to do that seems like unnecessary cognitive overhead.

@gaearon
Copy link
Member Author

gaearon commented Feb 28, 2015

Filed as eslint/eslint#1905

@sebmck
Copy link
Contributor

sebmck commented Apr 9, 2015

Closing this as eslint-plugin-react is now a thing.

@niftylettuce
Copy link

You need to use https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-uses-vars.md

@ianchanning
Copy link

Just in case anyone comes here as I did, because they've got a bunch of ESLinting JSX errors, there is an excellent Dan Abramov blog post which has some great guidance on how to get ESLint + Babel + JSX/React up and working as well as the SublimeText plugins. It has an very useful final image on a good set of base settings for the .eslintrc file for integrating eslint-plugin-react:

image

@a8568730
Copy link

a8568730 commented Apr 26, 2017

I had same problem.
I followed the document, and updated .eslintrc.json as below:

{
...
"extends": [
        "eslint:recommended",
        "plugin:react/recommended"
    ],
...
}

@Hemant-Synerzip
Copy link

"plugin:react/recommended"

Thank You. It worked for me. Great!!!!!!

@sky-gg
Copy link

sky-gg commented Oct 11, 2018

修改.eslintrc. 文件

"rules": {
        //...
        "react/jsx-uses-react": 1,
    }

@tomski80
Copy link

"rules": { //... "react/jsx-uses-react": 1, }

Thanks, this worked !

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

No branches or pull requests

10 participants