Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix migrate command #1495

Merged
merged 15 commits into from
Apr 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions packages/migrate/src/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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"));
Expand Down Expand Up @@ -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]);
Expand Down
42 changes: 42 additions & 0 deletions packages/utils/__tests__/run-prettier.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
48 changes: 20 additions & 28 deletions packages/utils/src/run-prettier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
2 changes: 1 addition & 1 deletion test/init/generator/init-inquirer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion test/loader/loader.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
7 changes: 7 additions & 0 deletions test/migrate/config/bad-webpack.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/* eslint-disable */

module.exports = {
output: {
badOption: true,
},
};
86 changes: 86 additions & 0 deletions test/migrate/config/migrate-config.test.js
Original file line number Diff line number Diff line change
@@ -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();
});
});
32 changes: 32 additions & 0 deletions test/migrate/config/webpack.config.js
Original file line number Diff line number Diff line change
@@ -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'],
},
},
],
},
};
Loading