Skip to content
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

Only lints and shows errors for most recently saved file in watch mode #44

Closed
densk1 opened this issue Nov 15, 2020 · 11 comments
Closed

Comments

@densk1
Copy link

densk1 commented Nov 15, 2020

  • Operating System: OSX 10.15.4
  • Node Version: v12.18.0
  • NPM Version: 6.14.8
  • webpack Version: 5.4.0
  • eslint-webpack-plugin Version: 2.3.0

Expected Behavior

When in watch mode, each time I save a file, all files in the tree are linted.

Actual Behavior

When in Webpack watch mode, on start it lints and shows errors for both A.ts and file B.ts in the linting path. When I save A.ts it only checks and displays errors for A.ts. When I save B.ts, , it then only lints and shows errors for file B.

Code

// webpack.config.js
...
  plugins: [
    new CleanWebpackPlugin(),
    new ESLintPlugin({
      files: 'src/**/*',
      // formatter: 'codeframe', // table
      extensions: ['js', 'ts', 'tsx'],
      // cache: true,
      failOnError: false,
      failOnWarning: false,
      emitWarning: true,
    }),
  ],
...

How Do We Reproduce?

Switch between files with linting errors in them, and save each one while in watch mode on v2.3.0.

Fix / Work around

I have reverted back to v2.1.0

@densk1
Copy link
Author

densk1 commented Nov 15, 2020

Guessing, by reading changes, but is it possibly related to this:

apply(compiler) {
if (!this.options.lintDirtyModulesOnly) {
compiler.hooks.run.tapPromise(ESLINT_PLUGIN, this.run);
}
// TODO: Figure out want `compiler.watching` is and how to use it in Webpack5.
// From my testing of compiler.watch() ... compiler.watching is always
// undefined (webpack 4 doesn't define it either) I'm leaving it out
// for now.
compiler.hooks.watchRun.tapPromise(ESLINT_PLUGIN, this.run);
}

and this

async run(compiler) {
// Do not re-hook
if (
// @ts-ignore
compiler.hooks.thisCompilation.taps.find(
// @ts-ignore
({ name }) => name === ESLINT_PLUGIN
)

Blocking re-running of this.run for watch?

@jsg2021
Copy link
Contributor

jsg2021 commented Nov 17, 2020

There is a watch mode test. Could you make a test case that fails?

@jsg2021
Copy link
Contributor

jsg2021 commented Nov 17, 2020

Guessing, by reading changes, but is it possibly related to this:

apply(compiler) {
if (!this.options.lintDirtyModulesOnly) {
compiler.hooks.run.tapPromise(ESLINT_PLUGIN, this.run);
}
// TODO: Figure out want `compiler.watching` is and how to use it in Webpack5.
// From my testing of compiler.watch() ... compiler.watching is always
// undefined (webpack 4 doesn't define it either) I'm leaving it out
// for now.
compiler.hooks.watchRun.tapPromise(ESLINT_PLUGIN, this.run);
}

and this

async run(compiler) {
// Do not re-hook
if (
// @ts-ignore
compiler.hooks.thisCompilation.taps.find(
// @ts-ignore
({ name }) => name === ESLINT_PLUGIN
)

Blocking re-running of this.run for watch?

The run method is probably miss named. It's actually registering hooks to a compiler. For watch mode, that compiler lives for the duration of your watch. So those hooks are already present it skips and let's the original hooks process each compilation.

@densk1
Copy link
Author

densk1 commented Nov 17, 2020

There is a watch mode test. Could you make a test case that fails?

I've modified watch.test.js. expect(message).toEqual(expect.stringMatching('prefer-const')); fails in thirdPass(). It passes in secondPass() but the file has not changed.

// watch.test.js

import { join } from 'path';
import { writeFileSync } from 'fs';

import { removeSync } from 'fs-extra';

import pack from './utils/pack';

const target = join(__dirname, 'fixtures', 'watch-entry.js');
const target2 = join(__dirname, 'fixtures', 'watch-entry-2.js');
const targetExpectedPattern = expect.stringMatching(
  target.replace(/\\/g, '\\\\')
);

describe('watch', () => {
  afterEach(() => {
    removeSync(target);
  });

  it('should watch', (done) => {
    const compiler = pack('good');

    const watch = compiler.watch({}, (err, stats) => {
      watch.close();
      expect(err).toBeNull();
      expect(stats.hasWarnings()).toBe(false);
      expect(stats.hasErrors()).toBe(false);
      done();
    });
  });

  it('should watch with unique messages', (done) => {
    writeFileSync(target, 'var foo = stuff\n');

    let next = firstPass;
    const compiler = pack('watch');
    const watch = compiler.watch({}, (err, stats) => next(err, stats));

    function firstPass(err, stats) {
      expect(err).toBeNull();
      expect(stats.hasWarnings()).toBe(false);
      expect(stats.hasErrors()).toBe(true);
      const { errors } = stats.compilation;
      expect(errors.length).toBe(1);
      const [{ message }] = errors;
      expect(message).toEqual(targetExpectedPattern);
      expect(message).toEqual(expect.stringMatching('\\(3 errors,'));

      next = secondPass;

      writeFileSync(target2, "let bar = false;\n");
      writeFileSync(target, "const x = require('./watch-entry-2.js')\n\nconst foo = false;\n");
    }

    function secondPass(err, stats) {
      expect(err).toBeNull();
      expect(stats.hasWarnings()).toBe(false);
      expect(stats.hasErrors()).toBe(true);
      const { errors } = stats.compilation;
      expect(errors.length).toBe(1);
      const [{ message }] = errors;
      expect(message).toEqual(targetExpectedPattern);
      expect(message).toEqual(expect.stringMatching('no-unused-vars'));
      expect(message).toEqual(expect.stringMatching('prefer-const')); // `prefer-const` passes here
      expect(message).toEqual(expect.stringMatching('\\(4 errors,'));
      
      next = thirdPass;
      
      writeFileSync(target, "const x = require('./watch-entry-2.js')\nconst foo = 0\n");
    }
    
    function thirdPass(err, stats) {
      expect(err).toBeNull();
      expect(stats.hasWarnings()).toBe(false);
      expect(stats.hasErrors()).toBe(true);
      const { errors } = stats.compilation;
      expect(errors.length).toBe(1);
      const [{ message }] = errors;
      expect(message).toEqual(targetExpectedPattern);
      expect(message).toEqual(expect.stringMatching('no-unused-vars'));
      expect(message).toEqual(expect.stringMatching('prefer-const')); // `prefer-const` fails here
      expect(message).toEqual(expect.stringMatching('\\(1 error,'));

      next = finish;

      writeFileSync(
        target,
        '/* eslint-disable no-unused-vars */\nconst foo = false;\n'
      );
    }

    function finish(err, stats) {
      watch.close();
      expect(err).toBeNull();
      expect(stats.hasWarnings()).toBe(false);
      expect(stats.hasErrors()).toBe(false);
      done();
    }
  });
});

@refaelok
Copy link

I have the exact same issue.
I'm following this issue and waiting for the next fix.

@inker
Copy link

inker commented Nov 30, 2020

Still occurring in 2.4.0.

@ricardogobbosouza
Copy link
Collaborator

@inker
I haven't released 2.4.1 yet 😬

@jsg2021 jsg2021 mentioned this issue Nov 30, 2020
6 tasks
@jsg2021
Copy link
Contributor

jsg2021 commented Nov 30, 2020

@ricardogobbosouza I fixed one last edge case for this

@refaelok
Copy link

refaelok commented Dec 1, 2020

actually the issue was fixed but created another performance issue.
now for each change in some file the build open new nodejs process and take more memory from cpu, that mean while you cding and saving in big projects, you get performance issue, due to for 3 with interval of few seconds create performance issue.

@jsg2021
Copy link
Contributor

jsg2021 commented Dec 1, 2020

the thread pool is now disabled by default. As a bonus, the pool will only be used for the initial completion.

@inker
Copy link

inker commented Dec 2, 2020

Works in 2.4.1. Thanks.

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

No branches or pull requests

5 participants