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

cli(refactor): improve folder structure #371

Merged
merged 9 commits into from
Apr 2, 2018
4 changes: 3 additions & 1 deletion bin/process-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,9 @@ module.exports = function processOptions(yargs, argv) {
);
} else if (stats.hash !== lastHash) {
lastHash = stats.hash;
const delimiter = outputOptions.buildDelimiter ? `${outputOptions.buildDelimiter}\n` : "";
const delimiter = outputOptions.buildDelimiter
? `${outputOptions.buildDelimiter}\n`
: "";
process.stdout.write("\n" + new Date() + "\n" + "\n");
process.stdout.write(`${stats.toString(outputOptions)}\n${delimiter}`);
if (argv.s) lastHash = null;
Expand Down
16 changes: 9 additions & 7 deletions bin/webpack.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@
(function() {
// wrap in IIFE to be able to use return

const resolveCwd = require("resolve-cwd");
// Local version replace global one
const localCLI = resolveCwd.silent("webpack-cli/bin/webpack");
if (localCLI && localCLI !== __filename) {
require(localCLI);
const importLocal = require("import-local");
// Prefer the local installation of webpack-cli
if (importLocal(__filename)) {
return;
}

Expand Down Expand Up @@ -48,7 +46,9 @@
return;
}

const yargs = require("yargs").usage(`webpack-cli ${require("../package.json").version}
const yargs = require("yargs").usage(`webpack-cli ${
require("../package.json").version
}

Usage: webpack-cli [options]
webpack-cli [options] --entry <entry> --output <output>
Expand Down Expand Up @@ -482,7 +482,9 @@ For more information, see https://webpack.js.org/api/cli/.`);
} else if (stats.hash !== lastHash) {
lastHash = stats.hash;
const statsString = stats.toString(outputOptions);
const delimiter = outputOptions.buildDelimiter ? `${outputOptions.buildDelimiter}\n` : "";
const delimiter = outputOptions.buildDelimiter
? `${outputOptions.buildDelimiter}\n`
: "";
if (statsString) stdout.write(`${statsString}\n${delimiter}`);
}
if (!options.watch && stats.hasErrors()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const { LoaderGenerator } = require("../generators/loader-generator");
* Runs a yeoman generator to create a new webpack loader project
* @returns {void}
*/

function loaderCreator() {
const env = yeoman.createEnv();
const generatorName = "webpack-loader-generator";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
const yeoman = require("yeoman-environment");
const PluginGenerator = require("../generators/plugin-generator").PluginGenerator;
const { PluginGenerator } = require("../generators/plugin-generator");

/**
* Runs a yeoman generator to create a new webpack plugin project
* @returns {void}
*/

function pluginCreator() {
const env = yeoman.createEnv();
const generatorName = "webpack-plugin-generator";
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/info.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module.exports = function info() {
Binaries: ["Node", "Yarn", "npm"],
Browsers: ["Chrome", "Firefox", "Safari"],
npmPackages: "*webpack*",
npmGlobalPackages: ["webpack", "webpack-cli"],
npmGlobalPackages: ["webpack", "webpack-cli"]
})
);
};
11 changes: 7 additions & 4 deletions lib/commands/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,17 @@ const modifyHelper = require("../utils/modify-config-helper");
* First function to be called after running the init flag. This is a check,
* if we are running the init command with no arguments or if we got dependencies
*
* @param {Object} pkg - packages included when running the init command
* @param {Array} args - array of arguments such as
* packages included when running the init command
* @returns {Function} creator/npmPackagesExists - returns an installation of the package,
* followed up with a yeoman instance of that if there's packages. If not, it creates a defaultGenerator
*/

module.exports = function initializeInquirer(pkg) {
if (pkg.length === 0) {
module.exports = function initializeInquirer(...args) {
const packages = args.slice(3);

if (packages.length === 0) {
return modifyHelper("init", defaultGenerator);
}
return npmPackagesExists(pkg);
return npmPackagesExists(packages);
};
94 changes: 68 additions & 26 deletions lib/commands/migrate.js
Original file line number Diff line number Diff line change
@@ -1,42 +1,83 @@
"use strict";

const fs = require("fs");
const path = require("path");
const chalk = require("chalk");
const diff = require("diff");
const inquirer = require("inquirer");
const PLazy = require("p-lazy");
const Listr = require("listr");

const validate = require("webpack").validate;
const WebpackOptionsValidationError = require("webpack")
.WebpackOptionsValidationError;
const { validate } = require("webpack");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we merge this together?

const { WebpackOptionsValidationError } = require("webpack");

const runPrettier = require("../utils/run-prettier");

/**
*
* Runs migration on a given configuration using AST's and promises
* to sequentially transform a configuration file.
*
* @param {String} currentConfigPath - Location of the configuration to be migrated
* @param {String} outputConfigPath - Location to where the configuration should be written
* @param {Object} options - Any additional options regarding code style of the written configuration
*
* Runs migration on a given configuration using AST's and promises
* to sequentially transform a configuration file.
*
* @param {Array} args - Migrate arguments such as input and
* output path
* @returns {Function} Runs the migration using the 'runMigrate'
* function.
*/

* @returns {Promise} Runs the migration using a promise that will throw any errors during each transform
* or output if the user decides to abort the migration
*/
module.exports = function migrate(...args) {
const filePaths = args.slice(3);
if (!filePaths.length) {
const errMsg = "\n ✖ Please specify a path to your webpack config \n ";
console.error(chalk.red(errMsg));
return;
}
const currentConfigPath = path.resolve(process.cwd(), filePaths[0]);
let outputConfigPath;
if (!filePaths[1]) {
return inquirer
.prompt([
{
type: "confirm",
name: "confirmPath",
message:
"Migration output path not specified. " +
"Do you want to use your existing webpack " +
"configuration?",
default: "Y"
}
])
.then(ans => {
if (!ans["confirmPath"]) {
console.error(chalk.red("✖ ︎Migration aborted due no output path"));
return;
}
outputConfigPath = path.resolve(process.cwd(), filePaths[0]);
return runMigration(currentConfigPath, outputConfigPath);
})
.catch(err => {
console.error(err);
});
}
outputConfigPath = path.resolve(process.cwd(), filePaths[1]);
return runMigration(currentConfigPath, outputConfigPath);
};

module.exports = function migrate(
currentConfigPath,
outputConfigPath,
options
) {
const recastOptions = Object.assign(
{
quote: "single"
},
options
);
/**
*
* Runs migration on a given configuration using AST's and promises
* to sequentially transform a configuration file.
*
* @param {String} currentConfigPath - input path for config
* @param {String} outputConfigPath - output path for config
* @returns {Promise} Runs the migration using a promise that
* will throw any errors during each transform or output if the
* user decides to abort the migration
*/

function runMigration(currentConfigPath, outputConfigPath) {
const recastOptions = {
quote: "single"
};
const tasks = new Listr([
{
title: "Reading webpack config",
Expand Down Expand Up @@ -145,8 +186,9 @@ module.exports = function migrate(
});
})
.catch(err => {
console.log(chalk.red("✖ ︎Migration aborted due to some errors"));
const errMsg = "\n ✖ ︎Migration aborted due to some errors: \n";
console.error(chalk.red(errMsg));
console.error(err);
process.exitCode = 1;
});
};
}
15 changes: 7 additions & 8 deletions lib/generators/init-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,7 @@ module.exports = class InitGenerator extends Generator {
constructor(args, opts) {
super(args, opts);
this.isProd = false;
this.dependencies = [
"webpack",
"webpack-cli",
"uglifyjs-webpack-plugin"
];
this.dependencies = ["webpack", "webpack-cli", "uglifyjs-webpack-plugin"];
this.configuration = {
config: {
webpackOptions: {},
Expand Down Expand Up @@ -119,7 +115,9 @@ module.exports = class InitGenerator extends Generator {
Confirm("prodConfirm", "Are you going to use this in production?")
]);
})
.then(prodConfirmAnswer => this.isProd = prodConfirmAnswer["prodConfirm"])
.then(
prodConfirmAnswer => (this.isProd = prodConfirmAnswer["prodConfirm"])
)
.then(() => {
return this.prompt([
Confirm("babelConfirm", "Will you be using ES2015?")
Expand Down Expand Up @@ -398,8 +396,9 @@ module.exports = class InitGenerator extends Generator {
)
])
.then(nameTypeAnswer => {
this.configuration.config.configName = nameTypeAnswer["nameType"].length ?
nameTypeAnswer["nameType"] : defaultName;
this.configuration.config.configName = nameTypeAnswer["nameType"].length
? nameTypeAnswer["nameType"]
: defaultName;
})
.then(() => {
asyncNamePrompt();
Expand Down
3 changes: 2 additions & 1 deletion lib/generators/loader-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ function makeLoaderName(name) {
* @class LoaderGenerator
* @extends {Generator}
*/

const LoaderGenerator = webpackGenerator(
[
{
Expand All @@ -36,7 +37,7 @@ const LoaderGenerator = webpackGenerator(
validate: str => str.length > 0
}
],
path.join(__dirname, "../generate-loader/templates"),
path.resolve(__dirname, "..", "generate-loader"),
[
"src/cjs.js.tpl",
"test/test-utils.js.tpl",
Expand Down
2 changes: 1 addition & 1 deletion lib/generators/loader-generator.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"use strict";

const makeLoaderName = require("./loader-generator").makeLoaderName;
const { makeLoaderName } = require("./loader-generator");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love these changes ❤️


describe("makeLoaderName", () => {
it("should kebab-case loader name and append '-loader'", () => {
Expand Down
2 changes: 1 addition & 1 deletion lib/generators/plugin-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const PluginGenerator = webpackGenerator(
validate: str => str.length > 0
}
],
path.join(__dirname, "../generate-plugin/templates"),
path.resolve(__dirname, "..", "generate-plugin"),
[
"src/cjs.js.tpl",
"test/test-utils.js.tpl",
Expand Down
2 changes: 1 addition & 1 deletion lib/generators/webpack-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function webpackGenerator(
this.npmInstall(["webpack-defaults", "bluebird"], {
"save-dev": true
}).then(() => {
this.spawnCommand("npm", ["run", "webpack-defaults"]);
this.spawnCommand("npm", ["run", "defaults"]);
});
}
};
Expand Down
52 changes: 5 additions & 47 deletions lib/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
"use strict";

const path = require("path");

/**
*
* First function to be called after running a flag. This is a check,
Expand All @@ -14,50 +12,10 @@ const path = require("path");
*/

module.exports = function initialize(command, args) {
const popArgs = args ? args.slice(2).pop() : null;
switch (command) {
case "init": {
const initPkgs = args.slice(2).length === 1 ? [] : [popArgs];
//eslint-disable-next-line
return require("./commands/init.js")(initPkgs);
}
case "migrate": {
const filePaths = args.slice(2).length === 1 ? [] : [popArgs];
if (!filePaths.length) {
throw new Error("Please specify a path to your webpack config");
}
const inputConfigPath = path.resolve(process.cwd(), filePaths[0]);
//eslint-disable-next-line
return require("./commands/migrate.js")(inputConfigPath, inputConfigPath);
}
case "add": {
//eslint-disable-next-line
return require("./commands/add")();
}
case "remove": {
//eslint-disable-next-line
return require("./commands/remove")();
}
case "update": {
return require("./commands/update")();
}
case "serve": {
return require("./commands/serve").serve();
}
case "make": {
return require("./commands/make")();
}
case "generate-loader": {
return require("./generate-loader/index.js")();
}
case "generate-plugin": {
return require("./generate-plugin/index.js")();
}
case "info": {
return require("./commands/info.js")();
}
default: {
throw new Error(`Unknown command ${command} found`);
}
if (!command) {
throw new Error(`Unknown command ${command} found`);
} else if (command === "serve") {
return require(`./commands/${command}`).serve();
}
return require(`./commands/${command}`)(...args);
Copy link
Contributor

@ematipico ematipico Mar 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would throw an error no file found. Are we OK with this? Or do we want to throw a better error message?
This is more as a comment, not a blocking change

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check for require and then doing this would make sense

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, we've defined all the commands in webpack/bin/webpack.js, rest will the webpack file take care of. What do you mean with check for require?

Copy link
Member

@sendilkumarn sendilkumarn Apr 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like if(require(...)) and do check or pass in (..args)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we put this require request in try catch block? and in catch block throw new Error stating that command not found, otherwise the error thrown will be that module was not found.

};
8 changes: 7 additions & 1 deletion lib/init/transformations/top-scope/top-scope.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@

const defineTest = require("../../../utils/defineTest");

defineTest(__dirname, "top-scope", "top-scope-0", ["const test = 'me';"], "init");
defineTest(
__dirname,
"top-scope",
"top-scope-0",
["const test = 'me';"],
"init"
);
defineTest(
__dirname,
"top-scope",
Expand Down
8 changes: 3 additions & 5 deletions lib/migrate/loaderOptionsPlugin/loaderOptionsPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,9 @@ module.exports = function(j, ast) {
// If there is debug: true, set debug: true in the plugin
if (ast.find(j.Identifier, { name: "debug" }).size()) {
loaderOptions.debug = true;
ast
.find(j.Identifier, { name: "debug" })
.forEach(p => {
p.parent.prune();
});
ast.find(j.Identifier, { name: "debug" }).forEach(p => {
p.parent.prune();
});
}

// If there is UglifyJsPlugin, set minimize: true
Expand Down
Loading