From 70ba7994aa73e9b1d60d5cffc1c9b54edb9092f4 Mon Sep 17 00:00:00 2001 From: evilebottnawi Date: Sun, 11 Oct 2020 19:51:38 +0300 Subject: [PATCH 1/6] fix: avoid unnecessary stringify --- packages/webpack-cli/lib/utils/Compiler.js | 59 +++++++++------------- 1 file changed, 25 insertions(+), 34 deletions(-) diff --git a/packages/webpack-cli/lib/utils/Compiler.js b/packages/webpack-cli/lib/utils/Compiler.js index 6c46b47c7b4..256d40923bf 100644 --- a/packages/webpack-cli/lib/utils/Compiler.js +++ b/packages/webpack-cli/lib/utils/Compiler.js @@ -55,49 +55,38 @@ class Compiler { }); } - generateOutput(outputOptions, stats) { - logger.raw(`${stats.toString(this.compilerOptions.stats)}\n`); - if (outputOptions.watch) { - logger.info('watching files for updates...'); - } - } - - compilerCallback(err, stats, lastHash, options, outputOptions) { - const statsErrors = []; - - if (!outputOptions.watch || err) { - // Do not keep cache anymore - this.compiler.purgeInputFileSystem(); - } - if (err) { + compilerCallback(error, stats, lastHash, options, outputOptions) { + if (error) { lastHash = null; - logger.error(err.stack || err); + logger.error(error); process.exit(1); } + if (!outputOptions.watch && stats.hasErrors()) { process.exitCode = 1; } - if (outputOptions.json === true) { - process.stdout.write(JSON.stringify(stats.toJson(outputOptions), null, 2) + '\n'); - } else if (stats.hash !== lastHash) { + + if (stats.hash !== lastHash) { lastHash = stats.hash; - if (stats.compilation && stats.compilation.errors.length !== 0) { - const errors = stats.compilation.errors; - errors.forEach((statErr) => { - const errLoc = statErr.module ? statErr.module.resource : null; - statsErrors.push({ name: statErr.message, loc: errLoc }); - }); - } - const JSONStats = JSON.stringify(stats.toJson(outputOptions), null, 2); - if (typeof outputOptions.json === 'string') { + + if (outputOptions.json === true) { + process.stdout.write(JSON.stringify(stats.toJson(outputOptions), null, 2) + '\n'); + } else if (typeof outputOptions.json === 'string') { + const JSONStats = JSON.stringify(stats.toJson(outputOptions), null, 2); + try { writeFileSync(outputOptions.json, JSONStats); logger.success(`stats are successfully stored as json to ${outputOptions.json}`); } catch (err) { logger.error(err); } + } else { + logger.raw(`${stats.toString(this.compilerOptions.stats)}\n`); + } + + if (outputOptions.watch) { + logger.info('watching files for updates...'); } - return this.generateOutput(outputOptions, stats, statsErrors); } } @@ -107,12 +96,14 @@ class Compiler { await this.compiler.run((err, stats) => { if (this.compiler.close) { this.compiler.close(() => { - const content = this.compilerCallback(err, stats, lastHash, options, outputOptions); - resolve(content); + this.compilerCallback(err, stats, lastHash, options, outputOptions); + + resolve(); }); } else { - const content = this.compilerCallback(err, stats, lastHash, options, outputOptions); - resolve(content); + this.compilerCallback(err, stats, lastHash, options, outputOptions); + + resolve(); } }); }); @@ -120,7 +111,7 @@ class Compiler { async invokeWatchInstance(lastHash, options, outputOptions, watchOptions) { return this.compiler.watch(watchOptions, (err, stats) => { - return this.compilerCallback(err, stats, lastHash, options, outputOptions); + this.compilerCallback(err, stats, lastHash, options, outputOptions); }); } From f3d031065904accd8d26d74414781825fa72a1dd Mon Sep 17 00:00:00 2001 From: evilebottnawi Date: Sun, 11 Oct 2020 21:01:30 +0300 Subject: [PATCH 2/6] tests: errors/warnings --- .gitignore | 1 + test/build-errors/errors.test.js | 52 +++++++++++++++++++++++++++ test/build-errors/src/index.js | 3 ++ test/build-warning/src/index.js | 9 +++++ test/build-warning/warnings.test.js | 52 +++++++++++++++++++++++++++ test/json/json.test.js | 55 ++++++++++++----------------- 6 files changed, 140 insertions(+), 32 deletions(-) create mode 100644 test/build-errors/errors.test.js create mode 100644 test/build-errors/src/index.js create mode 100644 test/build-warning/src/index.js create mode 100644 test/build-warning/warnings.test.js diff --git a/.gitignore b/.gitignore index efdfb260487..5b242bf613f 100644 --- a/.gitignore +++ b/.gitignore @@ -65,3 +65,4 @@ test/**/**/binary/** test/**/dist test/**/**/dist test/**/**/**/dist +test/**/stats.json diff --git a/test/build-errors/errors.test.js b/test/build-errors/errors.test.js new file mode 100644 index 00000000000..dc06684dee6 --- /dev/null +++ b/test/build-errors/errors.test.js @@ -0,0 +1,52 @@ +'use strict'; +const { run } = require('../utils/test-utils'); +const { stat, readFile } = require('fs'); +const { resolve } = require('path'); + +describe('warnings', () => { + it('should output by default', () => { + const { stdout, exitCode } = run(__dirname); + + expect(stdout).toMatch(/ERROR in/); + expect(stdout).toMatch(/Error: Can't resolve/); + expect(exitCode).toBe(1); + }); + + it('should output JSON with the "json" flag', () => { + const { stdout, exitCode } = run(__dirname, ['--json']); + + expect(() => JSON.parse(stdout)).not.toThrow(); + expect(exitCode).toBe(1); + + const json = JSON.parse(stdout); + + expect(json['hash']).toBeDefined(); + expect(json['errorsCount']).toBe(1); + expect(json['errors'][0].message).toMatch(/Can't resolve/); + }); + + it('should store json to a file', (done) => { + const { stdout, exitCode } = run(__dirname, ['--json', 'stats.json']); + + expect(stdout).toContain('stats are successfully stored as json to stats.json'); + expect(exitCode).toBe(1); + + stat(resolve(__dirname, './stats.json'), (err, stats) => { + expect(err).toBe(null); + expect(stats.isFile()).toBe(true); + + readFile(resolve(__dirname, 'stats.json'), 'utf-8', (error, data) => { + expect(error).toBe(null); + expect(() => JSON.parse(data)).not.toThrow(); + + const json = JSON.parse(data); + + expect(json['hash']).toBeDefined(); + expect(json['errorsCount']).toBe(1); + expect(json['errors'][0].message).toMatch(/Can't resolve/); + + done(); + }); + }); + }); +}); diff --git a/test/build-errors/src/index.js b/test/build-errors/src/index.js new file mode 100644 index 00000000000..a75a3dc05f9 --- /dev/null +++ b/test/build-errors/src/index.js @@ -0,0 +1,3 @@ +import unknown from './unknown.mjs'; + +export default unknown diff --git a/test/build-warning/src/index.js b/test/build-warning/src/index.js new file mode 100644 index 00000000000..6d694816c32 --- /dev/null +++ b/test/build-warning/src/index.js @@ -0,0 +1,9 @@ +let module; + +try { + module = require('unknown'); +} catch (e) { + // Ignore +} + +export default module diff --git a/test/build-warning/warnings.test.js b/test/build-warning/warnings.test.js new file mode 100644 index 00000000000..39af9cf81b4 --- /dev/null +++ b/test/build-warning/warnings.test.js @@ -0,0 +1,52 @@ +'use strict'; +const { run } = require('../utils/test-utils'); +const { stat, readFile } = require('fs'); +const { resolve } = require('path'); + +describe('errors', () => { + it('should output by default', () => { + const { stdout, exitCode } = run(__dirname); + + expect(stdout).toMatch(/WARNING in/); + expect(stdout).toMatch(/Error: Can't resolve/); + expect(exitCode).toBe(0); + }); + + it('should output JSON with the "json" flag', () => { + const { stdout, exitCode } = run(__dirname, ['--json']); + + expect(() => JSON.parse(stdout)).not.toThrow(); + expect(exitCode).toBe(0); + + const json = JSON.parse(stdout); + + expect(json['hash']).toBeDefined(); + expect(json['warningsCount']).toBe(1); + expect(json['warnings'][0].message).toMatch(/Can't resolve/); + }); + + it('should store json to a file', (done) => { + const { stdout, exitCode } = run(__dirname, ['--json', 'stats.json']); + + expect(stdout).toContain('stats are successfully stored as json to stats.json'); + expect(exitCode).toBe(0); + + stat(resolve(__dirname, './stats.json'), (err, stats) => { + expect(err).toBe(null); + expect(stats.isFile()).toBe(true); + + readFile(resolve(__dirname, 'stats.json'), 'utf-8', (error, data) => { + expect(error).toBe(null); + expect(() => JSON.parse(data)).not.toThrow(); + + const json = JSON.parse(data); + + expect(json['hash']).toBeDefined(); + expect(json['warningsCount']).toBe(1); + expect(json['warnings'][0].message).toMatch(/Can't resolve/); + + done(); + }); + }); + }); +}); diff --git a/test/json/json.test.js b/test/json/json.test.js index 699bcc851db..a248412bc0d 100644 --- a/test/json/json.test.js +++ b/test/json/json.test.js @@ -5,49 +5,40 @@ const { resolve } = require('path'); describe('json flag', () => { it('should return valid json', () => { - const { stdout } = run(__dirname, ['--json']); - - // helper function to check if JSON is valid - const parseJson = () => { - return JSON.parse(stdout); - }; - // check the JSON is valid. - expect(JSON.parse(stdout)['hash']).toBeTruthy(); - expect(JSON.parse(stdout)['version']).toBeTruthy(); - expect(JSON.parse(stdout)['time']).toBeTruthy(); - expect(parseJson).not.toThrow(); + const { stdout, exitCode } = run(__dirname, ['--json']); + + expect(() => JSON.parse(stdout)).not.toThrow(); + expect(exitCode).toBe(0); + + expect(JSON.parse(stdout)['hash']).toBeDefined(); }); it('should store json to a file', (done) => { - const { stdout } = run(__dirname, ['--json', 'stats.json']); + const { stdout, exitCode } = run(__dirname, ['--json', 'stats.json']); expect(stdout).toContain('stats are successfully stored as json to stats.json'); + expect(exitCode).toBe(0); + stat(resolve(__dirname, './stats.json'), (err, stats) => { expect(err).toBe(null); expect(stats.isFile()).toBe(true); - done(); - }); - readFile(resolve(__dirname, 'stats.json'), 'utf-8', (err, data) => { - expect(err).toBe(null); - expect(JSON.parse(data)['hash']).toBeTruthy(); - expect(JSON.parse(data)['version']).toBeTruthy(); - expect(JSON.parse(data)['time']).toBeTruthy(); - expect(() => JSON.parse(data)).not.toThrow(); - done(); + + readFile(resolve(__dirname, 'stats.json'), 'utf-8', (err, data) => { + expect(err).toBe(null); + expect(JSON.parse(data)['hash']).toBeTruthy(); + expect(JSON.parse(data)['version']).toBeTruthy(); + expect(JSON.parse(data)['time']).toBeTruthy(); + expect(() => JSON.parse(data)).not.toThrow(); + done(); + }); }); }); it('should return valid json with -j alias', () => { - const { stdout } = run(__dirname, ['-j']); - - // helper function to check if JSON is valid - const parseJson = () => { - return JSON.parse(stdout); - }; - // check the JSON is valid. - expect(JSON.parse(stdout)['hash']).toBeTruthy(); - expect(JSON.parse(stdout)['version']).toBeTruthy(); - expect(JSON.parse(stdout)['time']).toBeTruthy(); - expect(parseJson).not.toThrow(); + const { stdout, exitCode } = run(__dirname, ['-j']); + expect(() => JSON.parse(stdout)).not.toThrow(); + expect(exitCode).toBe(0); + + expect(JSON.parse(stdout)['hash']).toBeDefined(); }); }); From 969d885a328007ea13821d80f5c496130acc8d2c Mon Sep 17 00:00:00 2001 From: evilebottnawi Date: Sun, 11 Oct 2020 21:12:19 +0300 Subject: [PATCH 3/6] tests: fix --- test/build-errors/errors.test.js | 2 +- test/{build-warning => build-warnings}/src/index.js | 0 test/{build-warning => build-warnings}/warnings.test.js | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename test/{build-warning => build-warnings}/src/index.js (100%) rename test/{build-warning => build-warnings}/warnings.test.js (98%) diff --git a/test/build-errors/errors.test.js b/test/build-errors/errors.test.js index dc06684dee6..ef759bfe22a 100644 --- a/test/build-errors/errors.test.js +++ b/test/build-errors/errors.test.js @@ -3,7 +3,7 @@ const { run } = require('../utils/test-utils'); const { stat, readFile } = require('fs'); const { resolve } = require('path'); -describe('warnings', () => { +describe('errors', () => { it('should output by default', () => { const { stdout, exitCode } = run(__dirname); diff --git a/test/build-warning/src/index.js b/test/build-warnings/src/index.js similarity index 100% rename from test/build-warning/src/index.js rename to test/build-warnings/src/index.js diff --git a/test/build-warning/warnings.test.js b/test/build-warnings/warnings.test.js similarity index 98% rename from test/build-warning/warnings.test.js rename to test/build-warnings/warnings.test.js index 39af9cf81b4..2049cf41eff 100644 --- a/test/build-warning/warnings.test.js +++ b/test/build-warnings/warnings.test.js @@ -3,7 +3,7 @@ const { run } = require('../utils/test-utils'); const { stat, readFile } = require('fs'); const { resolve } = require('path'); -describe('errors', () => { +describe('warnings', () => { it('should output by default', () => { const { stdout, exitCode } = run(__dirname); From bfce721613a99a8be5cbc8f9015cb0f12b9c4536 Mon Sep 17 00:00:00 2001 From: evilebottnawi Date: Sun, 11 Oct 2020 21:22:51 +0300 Subject: [PATCH 4/6] tests: debug problems --- test/build-errors/errors.test.js | 2 ++ test/build-warnings/warnings.test.js | 1 + 2 files changed, 3 insertions(+) diff --git a/test/build-errors/errors.test.js b/test/build-errors/errors.test.js index ef759bfe22a..1707ed6bd44 100644 --- a/test/build-errors/errors.test.js +++ b/test/build-errors/errors.test.js @@ -41,6 +41,8 @@ describe('errors', () => { const json = JSON.parse(data); + console.log(json); + expect(json['hash']).toBeDefined(); expect(json['errorsCount']).toBe(1); expect(json['errors'][0].message).toMatch(/Can't resolve/); diff --git a/test/build-warnings/warnings.test.js b/test/build-warnings/warnings.test.js index 2049cf41eff..88bae582c62 100644 --- a/test/build-warnings/warnings.test.js +++ b/test/build-warnings/warnings.test.js @@ -28,6 +28,7 @@ describe('warnings', () => { it('should store json to a file', (done) => { const { stdout, exitCode } = run(__dirname, ['--json', 'stats.json']); + console.log(stdout); expect(stdout).toContain('stats are successfully stored as json to stats.json'); expect(exitCode).toBe(0); From 101929d90eb0200ba1d38a80c2dde19d12c4aad3 Mon Sep 17 00:00:00 2001 From: evilebottnawi Date: Sun, 11 Oct 2020 21:51:21 +0300 Subject: [PATCH 5/6] tests: fix --- test/build-errors/errors.test.js | 6 ++---- test/build-warnings/warnings.test.js | 7 ++++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/test/build-errors/errors.test.js b/test/build-errors/errors.test.js index 1707ed6bd44..2ad43f87f5b 100644 --- a/test/build-errors/errors.test.js +++ b/test/build-errors/errors.test.js @@ -21,7 +21,7 @@ describe('errors', () => { const json = JSON.parse(stdout); expect(json['hash']).toBeDefined(); - expect(json['errorsCount']).toBe(1); + expect(json['errors']).toHaveLength(1); expect(json['errors'][0].message).toMatch(/Can't resolve/); }); @@ -41,10 +41,8 @@ describe('errors', () => { const json = JSON.parse(data); - console.log(json); - expect(json['hash']).toBeDefined(); - expect(json['errorsCount']).toBe(1); + expect(json['errors']).toHaveLength(1); expect(json['errors'][0].message).toMatch(/Can't resolve/); done(); diff --git a/test/build-warnings/warnings.test.js b/test/build-warnings/warnings.test.js index 88bae582c62..a9ec06fe476 100644 --- a/test/build-warnings/warnings.test.js +++ b/test/build-warnings/warnings.test.js @@ -7,6 +7,8 @@ describe('warnings', () => { it('should output by default', () => { const { stdout, exitCode } = run(__dirname); + console.log(stdout); + expect(stdout).toMatch(/WARNING in/); expect(stdout).toMatch(/Error: Can't resolve/); expect(exitCode).toBe(0); @@ -21,14 +23,13 @@ describe('warnings', () => { const json = JSON.parse(stdout); expect(json['hash']).toBeDefined(); - expect(json['warningsCount']).toBe(1); + expect(json['warnings']).toHaveLength(1); expect(json['warnings'][0].message).toMatch(/Can't resolve/); }); it('should store json to a file', (done) => { const { stdout, exitCode } = run(__dirname, ['--json', 'stats.json']); - console.log(stdout); expect(stdout).toContain('stats are successfully stored as json to stats.json'); expect(exitCode).toBe(0); @@ -43,7 +44,7 @@ describe('warnings', () => { const json = JSON.parse(data); expect(json['hash']).toBeDefined(); - expect(json['warningsCount']).toBe(1); + expect(json['warnings']).toHaveLength(1); expect(json['warnings'][0].message).toMatch(/Can't resolve/); done(); From 6fa0abb9e1ad8c05c8f530d8809f0bc561cf3434 Mon Sep 17 00:00:00 2001 From: evilebottnawi Date: Sun, 11 Oct 2020 22:25:04 +0300 Subject: [PATCH 6/6] tests: fix --- .github/workflows/nodejs.yml | 2 +- test/build-errors/errors.test.js | 6 ++++-- test/build-warnings/src/index.js | 6 +++--- test/build-warnings/warnings.test.js | 8 ++++---- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index ee03f5b7f06..a5ce291b14d 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -57,7 +57,7 @@ jobs: matrix: os: [ubuntu-latest, windows-latest, macos-latest] node-version: [10.x, 12.x, 14.x] - webpack-version: [next, latest] + webpack-version: [webpack-4, latest] steps: - uses: actions/checkout@v2 diff --git a/test/build-errors/errors.test.js b/test/build-errors/errors.test.js index 2ad43f87f5b..23d13c1fe35 100644 --- a/test/build-errors/errors.test.js +++ b/test/build-errors/errors.test.js @@ -22,7 +22,8 @@ describe('errors', () => { expect(json['hash']).toBeDefined(); expect(json['errors']).toHaveLength(1); - expect(json['errors'][0].message).toMatch(/Can't resolve/); + // `message` for `webpack@5` + expect(json['errors'][0].message ? json['errors'][0].message : json['errors'][0]).toMatch(/Can't resolve/); }); it('should store json to a file', (done) => { @@ -43,7 +44,8 @@ describe('errors', () => { expect(json['hash']).toBeDefined(); expect(json['errors']).toHaveLength(1); - expect(json['errors'][0].message).toMatch(/Can't resolve/); + // `message` for `webpack@5` + expect(json['errors'][0].message ? json['errors'][0].message : json['errors'][0]).toMatch(/Can't resolve/); done(); }); diff --git a/test/build-warnings/src/index.js b/test/build-warnings/src/index.js index 6d694816c32..17d6eb1b468 100644 --- a/test/build-warnings/src/index.js +++ b/test/build-warnings/src/index.js @@ -1,9 +1,9 @@ -let module; +let obj; try { - module = require('unknown'); + obj = require('unknown'); } catch (e) { // Ignore } -export default module +export default obj diff --git a/test/build-warnings/warnings.test.js b/test/build-warnings/warnings.test.js index a9ec06fe476..6b880996301 100644 --- a/test/build-warnings/warnings.test.js +++ b/test/build-warnings/warnings.test.js @@ -7,8 +7,6 @@ describe('warnings', () => { it('should output by default', () => { const { stdout, exitCode } = run(__dirname); - console.log(stdout); - expect(stdout).toMatch(/WARNING in/); expect(stdout).toMatch(/Error: Can't resolve/); expect(exitCode).toBe(0); @@ -24,7 +22,8 @@ describe('warnings', () => { expect(json['hash']).toBeDefined(); expect(json['warnings']).toHaveLength(1); - expect(json['warnings'][0].message).toMatch(/Can't resolve/); + // `message` for `webpack@5` + expect(json['warnings'][0].message ? json['warnings'][0].message : json['warnings'][0]).toMatch(/Can't resolve/); }); it('should store json to a file', (done) => { @@ -45,7 +44,8 @@ describe('warnings', () => { expect(json['hash']).toBeDefined(); expect(json['warnings']).toHaveLength(1); - expect(json['warnings'][0].message).toMatch(/Can't resolve/); + // `message` for `webpack@5` + expect(json['warnings'][0].message ? json['warnings'][0].message : json['warnings'][0]).toMatch(/Can't resolve/); done(); });