fix: avoid unnecessary stringify (#1920)

webpack · Oct 11, 2020 · 5ef1e7b · 5ef1e7b
1 parent d32aeda
commit 5ef1e7b
Show file tree

Hide file tree

Showing 8 changed files with 170 additions and 67 deletions.
diff --git 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
@@ -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
@@ -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,20 +96,22 @@ 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();
                 }
             });
         });
     }
 
     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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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();
     });
 });
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		import unknown from './unknown.mjs';

		export default unknown