Create-react-app: Work around ESLint plugin discovery issue

[gaearon]

There seems to be no visible progress on https://github.com/eslint/eslint/issues/3458, and since it’s disproportionally affecting our users, maybe we should try doing some hack. The biggest issue is that IDEs can’t discover local ESLint and its plugins because they're inside node_modules/react-scripts/node_modules.

We could try a few things:

• Ship our own bin script called eslint that launches the "real" eslint with the right NODE_PATH or something (not sure if that would even work)
• Mess with npm internal state and copy all ESLint folders in node_modules one level higher on npm start. This way it won't work right after installing but should work after the project is first started. Maybe we could even make that a postinstall script for react-scripts.
• Something else crazy.

Regardless of the chosen solution, I’d rather do a hack and fix it up later than keep telling people to install global packages.

[stoikerty]

I see where you're coming from but wouldn't that leave somebody with a hack to deal with once you eject the app?

[just-boris]

Mess with npm internal state and copy all ESLint folders in node_modules one level higher on npm start

It can be done much simpler. Just exclude eslint-* packages from bundled dependencies, and they will bubble up to the top folder.

I assume that you support only npm3+ aren't you?

[gaearon]

wouldn't that leave somebody with a hack to deal with once you eject the app?

It should be removed as part of ejecting (we have a way to do that: // @remove-on-eject-begin and // @remove-on-eject-end).

I assume that you are supporting only npm3+ aren't you?

Nope, we support npm2 because that’s what ships with the stable Node right now.

[just-boris]

I expected this kind of answer, but as far as the issue is only about developer satisfaction, can we make it at least working only with npm3?

I am so tempted with this because with npm3 we can solve just with removing some code, rather than adding more weirdness.

[gaearon]

Removing ESLint from bundledDependencies will likely make installs slower.
There’s a reason we use them in the first place.

We could give it a try though.
Would you like to publish dummy react-scripts forks and measure the difference?

[just-boris]

Yes, I can try to play around it in next few days.

P.S. Are you sure that bundledDependencies make install faster? I have tried to measure it and figured out that npm still downloads some data for them anyway.

[stoikerty]

It's also worth saying that if anybody wants to help with the underlying issue, grind some coffee and head over to https://github.com/eslint/eslint/issues/3458 to read (and perhaps propose a solution) as fixing this would benefit everyone who uses eslint with plugins, including create-react-app.

[AsaAyers]

I'm using my own fork of linter-eslint that when it doesn't find a node_modules/eslint, it looks for node_modules/react-scripts/node_modules/eslint and uses it. Obviously this hack only works for Atom and only works for react-scripts.

I thought about adding another key to package.json to point to the version of eslint to use. Something like "eslintLocation": "./node_modules/react-scripts/node_modules/eslint". If editors read that key it wouldn't be specific to react-scripts like my current hack is.

[AsaAyers]

Ship our own bin script called eslint that launches the "real" eslint with the right NODE_PATH or something (not sure if that would even work)

This wont' work for linter-eslint because it doesn't execute the bin file. Since the plugin is already JavaScript and eslint is JavaScript, the plugin imports eslint code and runs it directly.

[just-boris]

@gaearon

Would you like to publish dummy react-scripts forks and measure the difference?

Done. I have posted my results there
https://github.com/facebookincubator/create-react-app/pull/41#issuecomment-252488540
It seems like something was changed since bundledDependencies were introduced in CRA.

[caesarsol]

What about makingeslint and friends install in the actual devDependencies of the newly created project?
The user would just have to use the project-local eslint binary, ant that's not a bad thing in my opinion.

[fson]

It was suggested in https://github.com/eslint/eslint/issues/3458#issuecomment-257161846 that it would be possible to depend on ESLint plugins by changing the config from an ESLint shareable config to an ESLint plugin. Plugins can define both rules and configurations. I'm going to give this a try and open a PR so we can see if this solution would work for us.

[fson]

993 implements the "configuration as a plugin" approach. I'm still having some issues with getting the plugin resolved correctly even with this approach, but it might related to how we use bundledDependencies.

[gaearon]

Feel free to remove bundledDeps.

[fson]

We need to figure out a better way to do the end to end tests. The cra.sh script currently adds our own packages as bundledDeps, but that causes problems with plugin discovery because own packages end up being in node_modules/react-scripts/node_modules and others are in node_modules so the latter don't see the former.

Any ideas how to run the e2e experience when developing in a way that uses all the local packages, but doesn't use bundledDeps?

[gaearon]

Just copy folders by hand after npm install? That is, with cp.

[fson]

Sounds like that could work. Or maybe symlink so you can even make changes without running the whole script from start again?

[gaearon]

¯_(ツ)_/¯ whatever works

[gaearon]

@fson Do we have any progress on this?

[fson]

@gaearon I commented about my concerns about the plugin approach here: https://github.com/facebookincubator/create-react-app/pull/993#issuecomment-259153941.

[gaearon]

This doesn't seem to be a big problem with npm 3 now that we removed bundledDependencies.
Closing.