mansona / lint-to-the-future

A modern way to progressively update your code to the best practices
54 stars 7 forks source link

Appears to not work in monorepos? (or with ESLint 7?) #15

Open NullVoxPopuli opened 2 years ago

NullVoxPopuli commented 2 years ago

Update, I'm unblocked for now, but it'd be nice for lint-to-the-future to detect monorepo, and use the eslint available to the monorepo as a whole, or locally to each project (some projects may not even use eslint (maybe? lol))


❯ yarn lint-to-the-future ignore
yarn run v1.22.17
$ /✂️/fc-4/node_modules/.bin/lint-to-the-future ignore
TypeError: Cannot read properties of undefined (reading 'split')
    at new Range (/✂️/fc-4/node_modules/semver/classes/range.js:32:8)
    at Object.intersects (/✂️/fc-4/node_modules/semver/ranges/intersects.js:3:8)
    at Object.ignoreAll (/✂️/fc-4/node_modules/lint-to-the-future-eslint/index.js:38:14)
    at main (/✂️/fc-4/node_modules/lint-to-the-future/cli.js:107:29)
    at Object.<anonymous> (/✂️/fc-4/node_modules/lint-to-the-future/cli.js:112:1)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I've traced that to this line: https://github.com/mansona/lint-to-the-future-eslint/blob/main/index.js#L38

  if (semver.intersects(eslintVersion, '8')) {

the eslintVersion I have is:

yarn list eslint
└─ eslint@7.32.0

I edited node_modules to see what was detected as eslintVersion. It was undefined. While I was in there, I noticed the code was checking:

  const currentPackageJSON = require(join(cwd, 'package.json'));

  const eslintVersion = currentPackageJSON.devDependencies.eslint;

so... thankfully, I have a bit of control here, so I can add eslint into devDeps of the root package.json. (it was recently remove and bundled with a lint-config package anyway, so :shrug:)

After adding eslint to my root package.json, and re-running lint-to-the-future, things seem to be moving, although quite slowly -- I'll open another issue

NullVoxPopuli commented 2 years ago

ah, this is also giving me trouble:

  let currentPackageJSON = require(join(process.cwd(), 'package.json'));

  let lttfPluginsNames = [
    ...Object.keys(currentPackageJSON.devDependencies || {}),
    ...Object.keys(currentPackageJSON.dependencies || {})
  ].filter(dep => dep.startsWith('lint-to-the-future-'));

I had tried to cd to a project so lint-to-the-future would run more quickly, but there are checks to prevent that. :thinking:

I've made these edits: lint-to-the-future

async function main() {
  // no need to check package.json
  let pluginNames = [
    'lint-to-the-future-eslint',
  ];

  let lttfPluginsNames = pluginNames.filter(name => {
    try {
      return require(name);
    } catch { /* module not found */ }
  });

  let lttfPlugins = lttfPluginsNames.map(name => ({
    import: importCwd(name),
    name,
  }));

  // ... (etc)

lint-to-the-future-eslint

async function ignoreAll(cwd = process.cwd()) {
  // I happen to know I'm not using esilnt 8
  // but if eslint exports a version at runtime, that could be used here
  // instead of trying to find the correct package.json
  let cli;
  let report;
  let results;

  const eslint = require('eslint');
  const { CLIEngine } = eslint;

  cli = new CLIEngine();
  report = cli.executeOnFiles([cwd]);
  results = report.results;

  const errors = results.filter((result) => result.errorCount > 0);

  errors.forEach(ignoreError);
}
lougreenwood commented 1 year ago

@NullVoxPopuli Do you know if this is still an issue? Thanks!

NullVoxPopuli commented 1 year ago

Haven't tried, sorry!

Sakethtadimeti commented 1 year ago

@NullVoxPopuli Do you know if this is still an issue? Thanks!

Yes. I've tried it today. Looks like we don't have monorepo support yet.