Skip to content

Commit

Permalink
Merge branch 'next' into fix/help-group
Browse files Browse the repository at this point in the history
  • Loading branch information
rishabh3112 authored Apr 29, 2020
2 parents a346435 + dfb9a91 commit 9b78c6d
Show file tree
Hide file tree
Showing 20 changed files with 294 additions and 193 deletions.
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/Bug_report.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ Steps to reproduce the behavior:

**Additional context**

<!-- Add any other context about the problem here like linking to an similar issue you might think is the cause. -->
<!-- Add any other context about the problem here like linking to a similar issue you might think is the cause. -->
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
1 change: 1 addition & 0 deletions packages/utils/__tests__/find-project-root/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// eslint-disable-next-line node/no-unpublished-import
import { findProjectRoot } from '../../../src/path-utils';
import { join } from 'path';

beforeAll(() => {
// This sets the project root as the root dir, needed since some tests change process cwd.
process.chdir(join(__dirname, '../../../../../'));
});

describe('findProjectRoot function', () => {
it('works correctly', () => {
/* when no directory is passed, it takes the current process working directory as starting point
which contains package.json thus it should be the project root */
const projectRoot = findProjectRoot();
expect(projectRoot).toEqual(process.cwd());
});

it('works correctly with a non-default dir', () => {
/* when passing a custom directory, it's used as the starting point and so it should yield the path
nearest package.json from the given directory */
const projectRoot = findProjectRoot(__dirname);
expect(projectRoot).toEqual(join(__dirname, '..'));
});
});
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');
});
});
6 changes: 3 additions & 3 deletions packages/utils/src/path-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ export function isLocalPath(str: string): boolean {

/**
* Find the root directory path of a project.
*
* @param {String} cwd - Any custom starting point to walk through directories
* @returns {String} Absolute path of the project root.
*/

export function findProjectRoot(): string {
const rootFilePath = findup('package.json');
export function findProjectRoot(cwd = process.cwd()): string {
const rootFilePath = findup('package.json', { cwd });
const projectRoot = path.dirname(rootFilePath);
return projectRoot;
}
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');
}
1 change: 0 additions & 1 deletion packages/webpack-cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ Options
-d, --dev Run development build
-p, --prod Run production build
--mode string Defines the mode to pass to webpack
--no-mode Sets mode="none" which disables any default behavior
-v, --version Get current version
--node-args string[] NodeJS flags
--stats type It instructs webpack on how to treat the stats
Expand Down
15 changes: 3 additions & 12 deletions packages/webpack-cli/lib/groups/ZeroConfigGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,12 @@ class ZeroConfigGroup extends GroupHelper {
if (process.env.NODE_ENV && (process.env.NODE_ENV === PRODUCTION || process.env.NODE_ENV === DEVELOPMENT)) {
return process.env.NODE_ENV;
} else {
// commander sets mode to false if --no-mode is specified
const noMode = this.args.mode === false;
if ((this.args.mode || noMode) && (this.args.dev || this.args.prod)) {
if (this.args.mode && (this.args.dev || this.args.prod)) {
logger.warn(
`You provided both ${this.args.mode ? 'mode' : 'no-mode'} and ${
`You provided both 'mode' and ${
this.args.prod ? '--prod' : '--dev'
} arguments. You should provide just one. "${this.args.mode ? 'mode' : 'no-mode'}" will be used`,
} arguments. You should provide just one. "mode" will be used`,
);
if (this.args.mode) {
return this.args.mode;
} else {
return NONE;
}
}

if (this.args.mode) {
Expand All @@ -46,8 +39,6 @@ class ZeroConfigGroup extends GroupHelper {
return PRODUCTION;
} else if (this.args.dev) {
return DEVELOPMENT;
} else if (noMode) {
return NONE;
}
return PRODUCTION;
}
Expand Down
8 changes: 0 additions & 8 deletions packages/webpack-cli/lib/utils/cli-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,14 +225,6 @@ module.exports = {
description: 'Defines the mode to pass to webpack',
link: 'https://webpack.js.org/concepts/#mode',
},
{
name: 'no-mode',
usage: '--no-mode',
type: Boolean,
group: ZERO_CONFIG_GROUP,
description: 'Sets mode="none" which disables any default behavior',
link: 'https://webpack.js.org/concepts/#mode',
},
{
name: 'version',
usage: '--version | --version <external-package>',
Expand Down
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

0 comments on commit 9b78c6d

Please sign in to comment.