Breaking: simplify config/plugin/parser resolution (fixes #10125)

[not-an-aardvark]

What is the purpose of this pull request? (put an "X" next to item)

[x] Add something to the core

What changes did you make? (Give an overview)

This change updates ESLint to load plugins relative to the user's project root, and other packages relative to where they're specified in a config. This simplifies ESLint's package-loading, resulting in fewer confusing errors about missing packages. It also fixes an existing design bug where ESLint would sometimes fail to load plugins in valid setups.

Implements eslint/rfcs#7, fixes #10125.

Is there anything you'd like reviewers to focus on?

Sorry about the large diff. I think about 75% of it is code churn in tests.

As an overview:

• lib/config/config-file.js and lib/config.js have been updated to load plugins from the user's project root, and configs/parsers from the config file where they're specified, as described in the RFC.
  • This required a bit of refactoring, primarily in lib/config/config-file.js.
  • This allows us to remove almost all of the "mocking" code from tests/lib/config/config-file.js and replace it with fixtures, because nothing depends on the location of the ESLint package itself anymore.
• Linter no longer depends on the filesystem at all, since the dynamic require behavior has been removed. I updated the Linter tests so that all of them can run in the browser environment (previously, the tests relating to custom parsers were skipped in browsers).
  • As a side-effect, the browserify build step when generating the website doesn't need to bundle espree separately because espree is no longer dynamically loaded.
  • Since parsers are now loaded outside of Linter, a missing parser is now a fatal error rather than a linting error.
• The config initializer code now checks the current version of ESLint, rather than the locally-installed version of ESLint, when deciding whether it should perform a local ESLint installation, because there is no longer a significant distinction between local and global ESLint installations. (edit: this change has been dropped, see Breaking: simplify config/plugin/parser resolution (fixes #10125) #11388 (comment))

[platinumazure]

Generally LGTM. Left some small comments.

I think it would be worthwhile to use a name like "_placeholder.js" or even "placeholder.js" to make it very clear that placeholder.js isn't some special keyword that they might want to use, but rather something completely fake and internal. But I don't feel strongly about it.

[codepunkt]

Looking forward to this. Thanks to everyone involved! 🥇

[aladdin-add]

load performance get ~3.5%+ 🚀

before:

➜  eslint git:(b9aabe34) npm run perf

> eslint@5.14.1 perf /Users/weiran/repo/github/eslint
> node Makefile.js perf
Loading:
  Load performance Run #1:  285.663945ms
  Load performance Run #2:  250.599011ms
  Load performance Run #3:  253.621801ms
  Load performance Run #4:  275.971296ms
  Load performance Run #5:  279.895436ms

  Load Performance median:  275.971296ms


Single File:
  CPU Speed is 2500 with multiplier 13000000
  Performance Run #1:  5767.848127ms
  Performance Run #2:  5594.752693ms
  Performance Run #3:  5930.292608ms
  Performance Run #4:  5797.659383ms
  Performance Run #5:  5003.96314ms

  Performance budget exceeded: 5767.848127ms (limit: 5200ms)


Multi Files (0 files):
  CPU Speed is 2500 with multiplier 39000000
  Performance Run #1:  11921.268776ms
  Performance Run #2:  11467.670329ms
  Performance Run #3:  11569.845004ms
  Performance Run #4:  11400.462975ms
  Performance Run #5:  11481.288755ms

  Performance budget ok:  11481.288755ms (limit: 15600ms)

after:

➜  eslint git:(config-loading-simplification) npm run perf

> eslint@5.14.1 perf /Users/weiran/repo/github/eslint
> node Makefile.js perf


Loading:
  Load performance Run #1:  276.741395ms
  Load performance Run #2:  266.120042ms
  Load performance Run #3:  264.211499ms
  Load performance Run #4:  278.164283ms
  Load performance Run #5:  255.247396ms

  Load Performance median:  266.120042ms


Single File:
  CPU Speed is 2500 with multiplier 13000000
  Performance Run #1:  5038.981713ms
  Performance Run #2:  5020.554556ms
  Performance Run #3:  4759.529149ms
  Performance Run #4:  4750.758541ms
  Performance Run #5:  4753.592061ms

  Performance budget ok:  4759.529149ms (limit: 5200ms)


Multi Files (0 files):
  CPU Speed is 2500 with multiplier 39000000
  Performance Run #1:  11402.664894ms
  Performance Run #2:  11326.0953ms
  Performance Run #3:  11965.02155ms
  Performance Run #4:  11544.740028ms
  Performance Run #5:  11692.876292ms

  Performance budget ok:  11544.740028ms (limit: 15600ms)

[aladdin-add]

exciting work! I just left a few minor questions/suggestions.

[mysticatea]

The config initializer code now checks the current version of ESLint, rather than the locally-installed version of ESLint, when deciding whether it should perform a local ESLint installation, because there is no longer a significant distinction between local and global ESLint installations.

I think that it still should use the local eslint version.

If the chosen shareable config has eslint in that peerDependencies, eslint --init command installs eslint into the local because it installs all peerDependencies automatically.

When that time, if the version of eslint that has been installed locally mismatched the version of the peerDependencies, eslint --init command shows an additional question to ask the user to upgrade or downgrade the local eslint. The getLocalESLintVersion() function is for that question.

[not-an-aardvark]

Is it a problem if the local version of ESLint has mismatched peerDependencies, if the user is using the global version of ESLint anyway?

It seems like everything would still work fine as long as they use the same version of ESLint to lint their code as they did to run --init.

[mysticatea]

The purpose of that check was to prevent eslint --init to downgrade existing ESLint silently (#8870).

Also, things such as editor extensions and npm-scripts may use the local eslint anyway, so it can be new confusion.

[not-an-aardvark]

I see. I'll revert the config-initializer changes.

[platinumazure]

LGTM, thanks! Just left a couple of small suggestions.

[platinumazure]

Just one small tweak, otherwise LGTM. Thanks!

[platinumazure]

LGTM, thanks!