From 5ef1e7b074390406b76cb3e25dd90f045e1bd8a2 Mon Sep 17 00:00:00 2001 From: Alexander Krasnoyarov Date: Sun, 11 Oct 2020 22:56:30 +0300 Subject: [PATCH] fix: avoid unnecessary stringify (#1920) --- .github/workflows/nodejs.yml | 2 +- .gitignore | 1 + packages/webpack-cli/lib/utils/Compiler.js | 59 +++++++++------------- test/build-errors/errors.test.js | 54 ++++++++++++++++++++ test/build-errors/src/index.js | 3 ++ test/build-warnings/src/index.js | 9 ++++ test/build-warnings/warnings.test.js | 54 ++++++++++++++++++++ test/json/json.test.js | 55 +++++++++----------- 8 files changed, 170 insertions(+), 67 deletions(-) create mode 100644 test/build-errors/errors.test.js create mode 100644 test/build-errors/src/index.js create mode 100644 test/build-warnings/src/index.js create mode 100644 test/build-warnings/warnings.test.js 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/.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/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); }); } diff --git a/test/build-errors/errors.test.js b/test/build-errors/errors.test.js new file mode 100644 index 00000000000..23d13c1fe35 --- /dev/null +++ b/test/build-errors/errors.test.js @@ -0,0 +1,54 @@ +'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(/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['errors']).toHaveLength(1); + // `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) => { + 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['errors']).toHaveLength(1); + // `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-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-warnings/src/index.js b/test/build-warnings/src/index.js new file mode 100644 index 00000000000..17d6eb1b468 --- /dev/null +++ b/test/build-warnings/src/index.js @@ -0,0 +1,9 @@ +let obj; + +try { + obj = require('unknown'); +} catch (e) { + // Ignore +} + +export default obj diff --git a/test/build-warnings/warnings.test.js b/test/build-warnings/warnings.test.js new file mode 100644 index 00000000000..6b880996301 --- /dev/null +++ b/test/build-warnings/warnings.test.js @@ -0,0 +1,54 @@ +'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(/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['warnings']).toHaveLength(1); + // `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) => { + 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['warnings']).toHaveLength(1); + // `message` for `webpack@5` + expect(json['warnings'][0].message ? json['warnings'][0].message : json['warnings'][0]).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(); }); });