diff --git a/packages/migrate/src/index.ts b/packages/migrate/src/index.ts index 2b0eb785ac4..54c3d756326 100644 --- a/packages/migrate/src/index.ts +++ b/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 => { 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 { ]) .then((ans: { confirmPath: boolean }): void | Promise => { 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]); diff --git a/packages/utils/__tests__/run-prettier.test.ts b/packages/utils/__tests__/run-prettier.test.ts new file mode 100644 index 00000000000..d39b3cfb774 --- /dev/null +++ b/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'); + }); +}); diff --git a/packages/utils/src/run-prettier.ts b/packages/utils/src/run-prettier.ts index 99b12858428..41eb61dcc6b 100644 --- a/packages/utils/src/run-prettier.ts +++ b/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'); } diff --git a/test/init/generator/init-inquirer.test.js b/test/init/generator/init-inquirer.test.js index 2c97c9fd71b..e2d3784eb84 100644 --- a/test/init/generator/init-inquirer.test.js +++ b/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); diff --git a/test/loader/loader.test.js b/test/loader/loader.test.js index 49e625a9a5e..9b5fd8de218 100644 --- a/test/loader/loader.test.js +++ b/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); diff --git a/test/migrate/config/bad-webpack.config.js b/test/migrate/config/bad-webpack.config.js new file mode 100644 index 00000000000..5bec4e124d8 --- /dev/null +++ b/test/migrate/config/bad-webpack.config.js @@ -0,0 +1,7 @@ +/* eslint-disable */ + +module.exports = { + output: { + badOption: true, + }, +}; diff --git a/test/migrate/config/migrate-config.test.js b/test/migrate/config/migrate-config.test.js new file mode 100644 index 00000000000..4c60ca6f940 --- /dev/null +++ b/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(); + }); +}); diff --git a/test/migrate/config/webpack.config.js b/test/migrate/config/webpack.config.js new file mode 100644 index 00000000000..b55e44c03e6 --- /dev/null +++ b/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'], + }, + }, + ], + }, +}; diff --git a/test/utils/test-utils.js b/test/utils/test-utils.js index d107e50de74..1b864c5e92b 100644 --- a/test/utils/test-utils.js +++ b/test/utils/test-utils.js @@ -41,7 +41,7 @@ function runWatch({ testCase, args = [], setOutput = true, outputKillStr = 'Time const watchPromise = execa(WEBPACK_PATH, argsWithOutput, { cwd, reject: false, - stdio: ENABLE_LOG_COMPILATION ? 'inherit' : 'pipe', + stdio: 'pipe', }); watchPromise.stdout.pipe( @@ -91,40 +91,75 @@ function runAndGetWatchProc(testCase, args = [], setOutput = true, input = '', f /** * runInitWithAnswers * @param {string} location location of current working directory + * @param {string[]} args CLI args to pass in * @param {string[]} answers answers to be passed to stdout for inquirer question + * @param {boolean} waitForOutput whether to wait for stdout before writing the next answer */ -const runPromptWithAnswers = (location, args, answers) => { +const runPromptWithAnswers = (location, args, answers, waitForOutput = true) => { const runner = runAndGetWatchProc(location, args, false, '', true); runner.stdin.setDefaultEncoding('utf-8'); - // Simulate answers by sending the answers after waiting for 2s - const simulateAnswers = answers.reduce((prevAnswer, answer) => { - return prevAnswer.then(() => { - return new Promise((resolvePromise) => { - setTimeout(() => { - runner.stdin.write(answer); - resolvePromise(); - }, 2000); - }); - }); - }, Promise.resolve()); + const delay = 2000; + let outputTimeout; + if (waitForOutput) { + let currentAnswer = 0; + const writeAnswer = () => { + if (currentAnswer < answers.length) { + runner.stdin.write(answers[currentAnswer]); + currentAnswer++; + } + }; - simulateAnswers.then(() => { - runner.stdin.end(); - }); + runner.stdout.pipe( + new Writable({ + write(chunk, encoding, callback) { + const output = chunk.toString('utf8'); + if (output) { + if (outputTimeout) { + clearTimeout(outputTimeout); + } + // we must receive new stdout, then have 2 seconds + // without any stdout before writing the next answer + outputTimeout = setTimeout(writeAnswer, delay); + } + + callback(); + }, + }), + ); + } else { + // Simulate answers by sending the answers every 2s + answers.reduce((prevAnswer, answer) => { + return prevAnswer.then(() => { + return new Promise((resolvePromise) => { + setTimeout(() => { + runner.stdin.write(answer); + resolvePromise(); + }, delay); + }); + }); + }, Promise.resolve()); + } return new Promise((resolve) => { const obj = {}; let stdoutDone = false; let stderrDone = false; + const complete = () => { + if (outputTimeout) { + clearTimeout(outputTimeout); + } + if (stdoutDone && stderrDone) { + runner.kill('SIGKILL'); + resolve(obj); + } + }; + runner.stdout.pipe( concat((result) => { stdoutDone = true; obj.stdout = result.toString(); - if (stderrDone) { - runner.kill('SIGKILL'); - resolve(obj); - } + complete(); }), ); @@ -132,10 +167,7 @@ const runPromptWithAnswers = (location, args, answers) => { concat((result) => { stderrDone = true; obj.stderr = result.toString(); - if (stdoutDone) { - runner.kill('SIGKILL'); - resolve(obj); - } + complete(); }), ); });