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

Warning: React version not specified in eslint-plugin-react settings. #1955

Closed
kud opened this Issue Aug 22, 2018 · 34 comments

Comments

@kud
Copy link

kud commented Aug 22, 2018

Hello,

Since I've updated this plugin, i've got a message like Warning: React version not specified in eslint-plugin-react settings..

This is true, I didn't specify the version of react, and this happens due to 8738e59#diff-4e51c5c91adee02670adaca34f7fd7fdR17

However, when I read the doc, I've got this:

"version": "15.0", // React version, default to the latest React stable release

It's said "by default, it uses the latest version of react".

So why displaying a warning if there's a default and this is what I want? as I keep my react often upgraded.

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Aug 22, 2018

(linking to your comment here: #1857 (comment))

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Aug 22, 2018

Just because there's a default doesn't mean you shouldn't always be providing an explicit value.

Repeating #1857 (comment):

i could see adding a separate option for "detect" (using the "version" as a fallback), which did require('react').version - want to open a PR that adds that?
(Note that it'd have to use process.cwd(), and the resolve package, to get the proper require path)

@kud

This comment has been minimized.

Copy link

kud commented Aug 22, 2018

Yes, it's true.

For the moment, I use this workaround: "version": "999.999.999".

@chrisdothtml

This comment has been minimized.

Copy link

chrisdothtml commented Aug 23, 2018

+1 for supporting latest as the value

@marcofugaro

This comment has been minimized.

Copy link
Contributor

marcofugaro commented Aug 28, 2018

Yeah please I don't want to update this config each time a new react comes out

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Aug 28, 2018

You have to update your package.json every time that happens; I’m not sure also updating eslintrc is much harder.

@marcofugaro

This comment has been minimized.

Copy link
Contributor

marcofugaro commented Aug 28, 2018

Yeaaah... I don't have it in a package.json, I have it in the eslint config shared for the team, and each time I would have to upgrade deps, update by hand, publish a new version of the tool.

Most importantly, I upgrade my dependencies using yarn upgrade-interactive --latest, not by hand. I (or other mantainers) might forget to update this version by hand.

@chrisdothtml

This comment has been minimized.

Copy link

chrisdothtml commented Aug 28, 2018

We also use a shared eslint config and this would make it quite annoying every upgrade. If "latest" isn't supported, everyone's just gonna use "999.999.999" instead, which could lead to issues in the future if this plugin assumes that property contains their actual React version

@kud

This comment has been minimized.

Copy link

kud commented Aug 29, 2018

#1955 (comment)

Well, I use ncu to update my packages, and yes it's not hard to update .eslintrc but you have to remember this and it's not something I want. :D

I'm using 999.999.999 for the moment.

@bhardy

This comment has been minimized.

Copy link

bhardy commented Aug 29, 2018

Comment removed and new issue created: #1963

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Aug 30, 2018

@bhardy that seems like a different issue; if you have the settings object with a react version you shouldn't get a warning at all.

@bhardy

This comment has been minimized.

Copy link

bhardy commented Aug 30, 2018

@ljharb would you like me to create a new issue?

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Aug 30, 2018

@bhardy yes please :-)

Timer added a commit to Timer/create-react-app that referenced this issue Sep 20, 2018

Always lint with latest React version
This is the best behavior so people have seamless upgrades to new React majors.

This is probably a terrible default and warning from the ESLint plugin, and we need to wait for yannickcr/eslint-plugin-react#1955 before changing this hardcoded behavior.

Closes #5034

Timer added a commit to Timer/create-react-app that referenced this issue Sep 20, 2018

Always lint with latest React version (facebook#5043)
This is the best behavior so people have seamless upgrades to new React majors.

This is probably a terrible default warning from the ESLint plugin, and we need to wait for yannickcr/eslint-plugin-react#1955 before changing this hardcoded behavior.

Closes facebook#5034
@mohsinulhaq

This comment has been minimized.

Copy link
Contributor

mohsinulhaq commented Sep 22, 2018

I tried {"version": "latest"}, and it's working fine. Why are we pushing for '999.999.999'?

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Sep 23, 2018

@mohsinulhaq it would be more explicit than relying on React never hitting v999

@mohsinulhaq

This comment has been minimized.

Copy link
Contributor

mohsinulhaq commented Sep 23, 2018

but there is no downside of using latest with regards to linting, right?

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Sep 23, 2018

The downside would be a lack of explicit configuration with respect to which React version you’re using, i suppose.

prichodko added a commit to strvcom/strv-react-scripts that referenced this issue Sep 26, 2018

Always lint with latest React version (#5043)
This is the best behavior so people have seamless upgrades to new React majors.

This is probably a terrible default warning from the ESLint plugin, and we need to wait for yannickcr/eslint-plugin-react#1955 before changing this hardcoded behavior.

Closes facebook#5034

zhDmitry pushed a commit to zhDmitry/create-react-app that referenced this issue Sep 30, 2018

Always lint with latest React version (facebook#5043)
This is the best behavior so people have seamless upgrades to new React majors.

This is probably a terrible default warning from the ESLint plugin, and we need to wait for yannickcr/eslint-plugin-react#1955 before changing this hardcoded behavior.

Closes facebook#5034
@anilanar

This comment has been minimized.

Copy link

anilanar commented Oct 10, 2018

I think this is an important bug. There are editor plugins that rely on exit code and stdout of eslint --fix/prettier-eslint for auto-formatting.

As a formatter, prettier-eslint must not log something to stdout when exit code is 0.

@bertho-zero

This comment has been minimized.

Copy link

bertho-zero commented Oct 10, 2018

@anilanar Even when the version is filled the warning appears with prettier-eslint, I fixed that and I wait for a response. (prettier/prettier-eslint#194)

@ljharb ljharb closed this in #1978 Oct 16, 2018

@manuelbieh

This comment has been minimized.

Copy link

manuelbieh commented Nov 10, 2018

Hmm had (or let's say: have) the same issue. Since I'm using an .eslintrc.js I was able to use the following workaround which seems to work. At least I don't see this warning anymore:

settings: {
    react: {
        version: require('./package.json').dependencies.react,
    },
},

🤷‍♂️

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Nov 10, 2018

@manuelbieh require('react/package.json').version; but once the next release goes out, you can use "detect".

@manuelbieh

This comment has been minimized.

Copy link

manuelbieh commented Nov 10, 2018

Ah right, even better.

Detect, also cool! Thanks 👍

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Dec 28, 2018

v7.12.0 is released.

@mohsinulhaq

This comment has been minimized.

Copy link
Contributor

mohsinulhaq commented Dec 28, 2018

Cool! Thanks

@jaydenseric

This comment has been minimized.

Copy link

jaydenseric commented Jan 2, 2019

I'm still seeing the warning, why is detect not the default?

screen shot 2019-01-02 at 12 24 08 pm

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Jan 2, 2019

Making that the default would be a breaking change.

@jaydenseric

This comment has been minimized.

Copy link

jaydenseric commented Jan 2, 2019

That's ok, just publish a new default in a new major version.

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Jan 2, 2019

When we get around to the next major, we surely will. There’s no hurry tho; you can maually set it for yourself.

@jaydenseric

This comment has been minimized.

Copy link

jaydenseric commented Jan 2, 2019

Glad it's on the roadmap, although it's suboptimal to get all eslint-plugin-react consumers worried about a warning, have them change their config, just to make their extra config redundant when the new version is out. Will users be warned their config is bloated and can be simplified once their setting is a default? Probably not. Then they will be maintaining junk code.

Useful defaults like this are a low hanging fruit; the sooner they are published the better for everyone.

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Jan 2, 2019

If they follow the link, they’ll see they can use “detect”.

And sure, we could warn them of that, but there’s zero harm from explicitly specifying a default.

@aczekajski

This comment has been minimized.

Copy link

aczekajski commented Jan 3, 2019

The detect option is missing in docs, it took me way too long to find this issue and thus an info about the new option.

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Jan 3, 2019

Good catch; a PR to add it to the docs would be appreciated.

@eeriee

This comment has been minimized.

Copy link

eeriee commented Jan 5, 2019

Add 'detect' to the setting but still encounter the warning.

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Jan 5, 2019

@eeriee if updating to the latest doesn’t fix it, please file a new issue with your config and your plugin version

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