fix: regression with migrate command

Author: knagaitsev

packages/migrate/src/index.ts

@@ -1,5 +1,5 @@
 import chalk from 'chalk';
-import diff from 'diff';
+import { Change, diffLines } from 'diff';
 import fs from 'fs';
 import inquirer from 'inquirer';
 import Listr from 'listr';
@@ -90,9 +90,9 @@ function runMigration(currentConfigPath: string, outputConfigPath: string): Prom
         .run()
         .then((ctx: Node): void | Promise<void> => {
             const result: string = ctx.ast.toSource(recastOptions);
-            const diffOutput: diff.Change[] = diff.diffLines(ctx.source, result);
+            const diffOutput: Change[] = diffLines(ctx.source, result);
 
-            diffOutput.forEach((diffLine: diff.Change): void => {
+            diffOutput.forEach((diffLine: Change): void => {
                 if (diffLine.added) {
                     process.stdout.write(chalk.green(`+ ${diffLine.value}`));
                 } else if (diffLine.removed) {
@@ -133,15 +133,11 @@ function runMigration(currentConfigPath: string, outputConfigPath: string): Prom
                             return;
                         }
 
-                        runPrettier(outputConfigPath, result, (err: object): void => {
-                            if (err) {
-                                throw err;
-                            }
-                        });
+                        runPrettier(outputConfigPath, result);
 
                         if (answer.confirmValidation) {
-                            const outputPath = await import(outputConfigPath);
-                            const webpackOptionsValidationErrors: string[] = validate(outputPath);
+                            const outputConfig = (await import(outputConfigPath)).default;
+                            const webpackOptionsValidationErrors: string[] = validate(outputConfig);
 
                             if (webpackOptionsValidationErrors.length) {
                                 console.error(chalk.red("\n✖ Your configuration validation wasn't successful \n"));
@@ -198,7 +194,7 @@ export default function migrate(...args: string[]): void | Promise<void> {
             ])
             .then((ans: { confirmPath: boolean }): void | Promise<void> => {
                 if (!ans.confirmPath) {
-                    console.error(chalk.red('✖ ︎Migration aborted due no output path'));
+                    console.error(chalk.red('✖ ︎Migration aborted due to no output path'));
                     return;
                 }
                 outputConfigPath = path.resolve(process.cwd(), filePaths[0]);

packages/utils/__tests__/run-prettier.test.ts

@@ -0,0 +1,42 @@
+'use strict';
+
+import fs from 'fs';
+import path from 'path';
+//eslint-disable-next-line node/no-extraneous-import
+import rimraf from 'rimraf';
+import { runPrettier } from '../src/run-prettier';
+
+const outputPath = path.join(__dirname, 'test-assets');
+const outputFile = path.join(outputPath, 'test.js');
+const stdoutSpy = jest.spyOn(process.stdout, 'write');
+
+describe('runPrettier', () => {
+    beforeEach(() => {
+        rimraf.sync(outputPath);
+        fs.mkdirSync(outputPath);
+        stdoutSpy.mockClear();
+    });
+
+    afterAll(() => {
+        rimraf.sync(outputPath);
+    });
+
+    it('should run prettier on JS string and write file', () => {
+        runPrettier(outputFile, 'console.log("1");console.log("2");');
+        expect(fs.existsSync(outputFile)).toBeTruthy();
+        const data = fs.readFileSync(outputFile, 'utf8');
+        expect(data).toContain("console.log('1');\n");
+
+        expect(stdoutSpy.mock.calls.length).toEqual(0);
+    });
+
+    it('prettier should fail on invalid JS, with file still written', () => {
+        runPrettier(outputFile, '"');
+        expect(fs.existsSync(outputFile)).toBeTruthy();
+        const data = fs.readFileSync(outputFile, 'utf8');
+        expect(data).toContain('"');
+
+        expect(stdoutSpy.mock.calls.length).toEqual(1);
+        expect(stdoutSpy.mock.calls[0][0]).toContain('WARNING: Could not apply prettier');
+    });
+});

packages/utils/src/run-prettier.ts

@@ -8,35 +8,27 @@ import prettier from 'prettier';
  *
  * @param {String} outputPath - Path to write the config to
  * @param {Node} source - AST to write at the given path
- * @param {Function} cb - executes a callback after execution if supplied
- * @returns {Void} Writes a file at given location and prints messages accordingly
+ * @returns {Void} Writes a file at given location
  */
 
-export function runPrettier(outputPath: string, source: string, cb?: Function): void {
-    function validateConfig(): void | Function {
-        let prettySource: string;
-        let error: object;
-        try {
-            prettySource = prettier.format(source, {
-                filepath: outputPath,
-                parser: 'babel',
-                singleQuote: true,
-                tabWidth: 1,
-                useTabs: true,
-            });
-        } catch (err) {
-            process.stdout.write(
-                `\n${chalk.yellow(
-                    `WARNING: Could not apply prettier to ${outputPath}` + ' due validation error, but the file has been created\n',
-                )}`,
-            );
-            prettySource = source;
-            error = err;
-        }
-        if (cb) {
-            return cb(error);
-        }
-        return fs.writeFileSync(outputPath, prettySource, 'utf8');
+export function runPrettier(outputPath: string, source: string): void {
+    let prettySource: string = source;
+    try {
+        prettySource = prettier.format(source, {
+            filepath: outputPath,
+            parser: 'babel',
+            singleQuote: true,
+            tabWidth: 1,
+            useTabs: true,
+        });
+    } catch (err) {
+        process.stdout.write(
+            `\n${chalk.yellow(
+                `WARNING: Could not apply prettier to ${outputPath}` + ' due validation error, but the file has been created\n',
+            )}`,
+        );
+        prettySource = source;
     }
-    return fs.writeFile(outputPath, source, 'utf8', validateConfig);
+
+    return fs.writeFileSync(outputPath, prettySource, 'utf8');
 }

test/init/generator/init-inquirer.test.js

@@ -22,7 +22,7 @@ describe('init', () => {
     });
 
     it('should scaffold when given answers', async () => {
-        const { stdout } = await runPromptWithAnswers(genPath, ['init'], ['N', ENTER, ENTER, ENTER, ENTER, ENTER, ENTER, ENTER]);
+        const { stdout } = await runPromptWithAnswers(genPath, ['init'], [`N${ENTER}`, ENTER, ENTER, ENTER, ENTER, ENTER, ENTER]);
 
         expect(stdout).toBeTruthy();
         expect(stdout).toContain(firstPrompt);

test/loader/loader.test.js

@@ -31,7 +31,7 @@ describe('loader command', () => {
     });
 
     it('should scaffold loader template with a given name', async () => {
-        const { stdout } = await runPromptWithAnswers(__dirname, ['loader'], [loaderName, ENTER]);
+        const { stdout } = await runPromptWithAnswers(__dirname, ['loader'], [`${loaderName}${ENTER}`]);
 
         expect(stdout).toContain(firstPrompt);
 

test/migrate/config/bad-webpack.config.js

@@ -0,0 +1,7 @@
+/* eslint-disable */
+
+module.exports = {
+    output: {
+        badOption: true,
+    },
+};

test/migrate/config/migrate-config.test.js

@@ -0,0 +1,86 @@
+'use strict';
+
+const fs = require('fs');
+const path = require('path');
+const rimraf = require('rimraf');
+const { run, runAndGetWatchProc, runPromptWithAnswers } = require('../../utils/test-utils');
+
+const ENTER = '\x0D';
+const outputDir = 'test-assets';
+const outputPath = path.join(__dirname, outputDir);
+const outputFile = `${outputDir}/updated-webpack.config.js`;
+const outputFilePath = path.join(__dirname, outputFile);
+
+describe('migrate command', () => {
+    beforeEach(() => {
+        rimraf.sync(outputPath);
+        fs.mkdirSync(outputPath);
+    });
+
+    afterAll(() => {
+        rimraf.sync(outputPath);
+    });
+
+    it('should warn if the source config file is not specified', () => {
+        const { stderr } = run(__dirname, ['migrate'], false);
+        expect(stderr).toContain('Please specify a path to your webpack config');
+    });
+
+    it('should prompt accordingly if an output path is not specified', () => {
+        const { stdout } = run(__dirname, ['migrate', 'webpack.config.js'], false);
+        expect(stdout).toContain('? Migration output path not specified');
+    });
+
+    it('should throw an error if the user refused to overwrite the source file and no output path is provided', async () => {
+        const { stderr } = await runAndGetWatchProc(__dirname, ['migrate', 'webpack.config.js'], false, 'n');
+        expect(stderr).toBe('✖ ︎Migration aborted due to no output path');
+    });
+
+    it('should prompt for config validation when an output path is provided', async () => {
+        const { stdout } = await runAndGetWatchProc(__dirname, ['migrate', 'webpack.config.js', outputFile], false, 'y');
+        // should show the diff of the config file
+        expect(stdout).toContain('rules: [');
+        expect(stdout).toContain('? Do you want to validate your configuration?');
+    });
+
+    it('should generate an updated config file when an output path is provided', async () => {
+        const { stdout, stderr } = await runPromptWithAnswers(
+            __dirname,
+            ['migrate', 'webpack.config.js', outputFile],
+            [ENTER, ENTER],
+            true,
+        );
+        expect(stdout).toContain('? Do you want to validate your configuration?');
+        // should show the diff of the config file
+        expect(stdout).toContain('rules: [');
+        expect(stderr).toBeFalsy();
+
+        expect(fs.existsSync(outputFilePath)).toBeTruthy();
+        // the output file should be a valid config file
+        const config = require(outputFilePath);
+        expect(config.module.rules).toEqual([
+            {
+                test: /\.js$/,
+                exclude: /node_modules/,
+
+                use: [
+                    {
+                        loader: 'babel-loader',
+
+                        options: {
+                            presets: ['@babel/preset-env'],
+                        },
+                    },
+                ],
+            },
+        ]);
+    });
+
+    it('should generate an updated config file and warn of an invalid webpack config', async () => {
+        const { stdout, stderr } = await runPromptWithAnswers(__dirname, ['migrate', 'bad-webpack.config.js', outputFile], [ENTER, ENTER]);
+        expect(stdout).toContain('? Do you want to validate your configuration?');
+        expect(stderr).toContain("configuration.output has an unknown property 'badOption'");
+
+        expect(fs.existsSync(outputFilePath)).toBeTruthy();
+    });
+});

test/migrate/config/webpack.config.js

@@ -0,0 +1,32 @@
+/* eslint-disable */
+const path = require('path');
+
+module.exports = {
+    entry: {
+        index: './src/index.js',
+        vendor: './src/vendor.js',
+    },
+
+    output: {
+        filename: '[name].[chunkhash].js',
+        chunkFilename: '[name].[chunkhash].js',
+        path: path.resolve(__dirname, 'dist'),
+    },
+
+    optimization: {
+        minimize: true
+    },
+
+    module: {
+        loaders: [
+            {
+                test: /\.js$/,
+                exclude: /node_modules/,
+                loader: 'babel',
+                query: {
+                    presets: ['@babel/preset-env'],
+                },
+            },
+        ],
+    },
+};