From 709bcb778f5d63111cdbe70b824042303d248fd4 Mon Sep 17 00:00:00 2001 From: Andrey Okonetchnikov Date: Thu, 23 Feb 2017 15:59:38 +0100 Subject: [PATCH 1/5] WIP on Listr integration --- lib/migrate.js | 89 +++++++++++++++++++++++++----------- lib/transformations/index.js | 7 ++- package.json | 1 + 3 files changed, 69 insertions(+), 28 deletions(-) diff --git a/lib/migrate.js b/lib/migrate.js index 1fefff503a8..d17de010e49 100644 --- a/lib/migrate.js +++ b/lib/migrate.js @@ -1,36 +1,71 @@ const fs = require('fs'); const diff = require('diff'); const chalk = require('chalk'); -const transform = require('./transformations').transform; +const jscodeshift = require('jscodeshift'); +const transformations = require('./transformations').transformations; const inquirer = require('inquirer'); +const Listr = require('listr'); -module.exports = (currentConfigPath, outputConfigPath) => { +const migrateTasks = function(ast) { + return Object.keys(transformations).map(key => { + const transform = transformations[key]; + return { + title: key, + task: () => new Promise((resolve, reject) => { + console.log(key); + try { + const res = (transform(jscodeshift, ast))(); + // setTimeout(resolve, 1000) + resolve(res); + } catch (err) { + reject(err); + } + }) + }; + }); +}; + +module.exports = function tranformFile(currentConfigPath, outputConfigPath, options) { + const recastOptions = Object.assign({ + quote: 'single' + }, options); let currentConfig = fs.readFileSync(currentConfigPath, 'utf8'); - const outputConfig = transform(currentConfig); - const diffOutput = diff.diffLines(currentConfig, outputConfig); - diffOutput.map(diffLine => { - if (diffLine.added) { - process.stdout.write(chalk.green(`+ ${diffLine.value}`)); - } else if (diffLine.removed) { - process.stdout.write(chalk.red(`- ${diffLine.value}`)); + const ast = jscodeshift(currentConfig); + const tasks = new Listr([ + { + title: 'Migrating config from v1 to v2', + task: () => new Listr(migrateTasks(ast)) } - }); - inquirer - .prompt([ - { - type: 'confirm', - name: 'confirmMigration', - message: 'Are you sure these changes are fine?', - default: 'Y' - } - ]) - .then(answers => { - if (answers['confirmMigration']) { - // TODO validate the config - fs.writeFileSync(outputConfigPath, outputConfig, 'utf8'); - process.stdout.write(chalk.green(`Congratulations! Your new webpack v2 config file is at ${outputConfigPath}`)); - } else { - process.stdout.write(chalk.yellow('Migration aborted')); - } + ]); + + tasks.run() + .then(() => { + const outputConfig = ast.toSource(recastOptions); + const diffOutput = diff.diffLines(currentConfig, outputConfig); + diffOutput.map(diffLine => { + if (diffLine.added) { + process.stdout.write(chalk.green(`+ ${diffLine.value}`)); + } else if (diffLine.removed) { + process.stdout.write(chalk.red(`- ${diffLine.value}`)); + } + }); + inquirer + .prompt([ + { + type: 'confirm', + name: 'confirmMigration', + message: 'Are you sure these changes are fine?', + default: 'Y' + } + ]) + .then(answers => { + if (answers['confirmMigration']) { + // TODO validate the config + fs.writeFileSync(outputConfigPath, outputConfig, 'utf8'); + process.stdout.write(chalk.green(`Congratulations! Your new webpack v2 config file is at ${outputConfigPath}`)); + } else { + process.stdout.write(chalk.yellow('Migration aborted')); + } + }); }); }; diff --git a/lib/transformations/index.js b/lib/transformations/index.js index 38d42103b11..b16e05ef4af 100644 --- a/lib/transformations/index.js +++ b/lib/transformations/index.js @@ -20,6 +20,10 @@ const transformations = { removeDeprecatedPluginsTransform }; +function transformSingleAST(ast, transformFunction) { + return transformFunction(jscodeshift, ast); +} + /* * @function transform * @@ -37,11 +41,12 @@ function transform(source, transforms, options) { quote: 'single' }, options); transforms = transforms || Object.keys(transformations).map(k => transformations[k]); - transforms.forEach(f => f(jscodeshift, ast)); + transforms.forEach(f => transformSingleAST(ast, f)); return ast.toSource(recastOptions); } module.exports = { transform, + transformSingleAST, transformations }; diff --git a/package.json b/package.json index 870a5c6e5b0..c477aa7b661 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "inquirer": "^2.0.0", "interpret": "^1.0.1", "jscodeshift": "^0.3.30", + "listr": "^0.11.0", "loader-utils": "^0.2.16", "lodash": "^4.17.4", "recast": "git://github.com/kalcifer/recast.git#bug/allowbreak", From ac3fc7ef2b16f080145194d84bee4d852372350b Mon Sep 17 00:00:00 2001 From: Andrey Okonetchnikov Date: Sun, 19 Mar 2017 14:53:41 +0100 Subject: [PATCH 2/5] tests: Added throwing webpack config fixture --- .../__testfixtures__/failing.js | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 lib/transformations/__testfixtures__/failing.js diff --git a/lib/transformations/__testfixtures__/failing.js b/lib/transformations/__testfixtures__/failing.js new file mode 100644 index 00000000000..95de24dcb82 --- /dev/null +++ b/lib/transformations/__testfixtures__/failing.js @@ -0,0 +1,81 @@ +var webpack = require('webpack'); +var nodeEnvironment = process.env.NODE_ENV +var _ = require('lodash'); + +var config = { + entry: { + 'lib': './app/index.js', + 'email': './app/email.js' + }, + plugins: [ + new webpack.DefinePlugin({ + 'INCLUDE_ALL_MODULES': function includeAllModulesGlobalFn(modulesArray, application) { + modulesArray.forEach(function executeModuleIncludesFn(moduleFn) { + moduleFn(application); + }); + }, + ENVIRONMENT: JSON.stringify(nodeEnvironment) + }) + ], + output: { + path: __dirname + '/app', + filename: 'bundle.js' + }, + resolve: { + root: __dirname + '/app' + }, + module: { + // preLoaders: [ + // { test: /\.js?$/, loader: 'eslint', exclude: /node_modules/ } + // ], + loaders: [ + { test: /\.js$/, exclude: /(node_modules)/, loader: 'babel' }, + { test: /\.html/, exclude: [/(node_modules)/, /src\/index\.html/], loader: 'html-loader' }, + { test: /\.s?css$/, loader: 'style!css!sass' }, + { test: /\.(png|jpg)$/, loader: 'url-loader?mimetype=image/png' } + ] + }, + // extra configuration options. + // eslint: { + // configFile: '.eslintrc.js' + // } +} + +switch (nodeEnvironment) { + case 'production': + config.plugins.push(new webpack.optimize.UglifyJsPlugin()); + case 'preproduction': + config.output.path = __dirname + '/dist'; + config.plugins.push(new webpack.optimize.DedupePlugin()); + config.plugins.push(new webpack.optimize.OccurenceOrderPlugin()); + + config.output.filename = '[name].js'; + + config.entry = { + 'lib': ['./app/index.js', 'angular', 'lodash'], + 'email': ['./app/email.js', 'angular'] + }; + + config.devtool = 'source-map'; + config.output.libraryTarget = 'commonjs2'; + + break; + + case 'test': + config.entry = './index.js'; + break; + + case 'development': + config.entry = { + 'lib': ['./app/index.js', 'webpack/hot/dev-server'], + 'email': ['./app/email.js', 'webpack/hot/dev-server'] + }; + config.output.filename = '[name].js'; + config.devtool = 'source-map'; + break; + + default: + console.warn('Unknown or Undefined Node Environment. Please refer to package.json for available build commands.'); +} + +module.exports = config; \ No newline at end of file From c319ebe49ad91898c8ce899a189c409289320437 Mon Sep 17 00:00:00 2001 From: Andrey Okonetchnikov Date: Sun, 19 Mar 2017 14:54:04 +0100 Subject: [PATCH 3/5] Promisify transform functions and use listr to show success/errors --- lib/migrate.js | 19 +++++-------- lib/transformations/defineTest.js | 8 +++--- lib/transformations/index.js | 47 ++++++++++++++++++++++--------- lib/transformations/index.test.js | 32 +++++++++++++-------- package.json | 1 + 5 files changed, 65 insertions(+), 42 deletions(-) diff --git a/lib/migrate.js b/lib/migrate.js index d17de010e49..d5829de7b97 100644 --- a/lib/migrate.js +++ b/lib/migrate.js @@ -11,16 +11,7 @@ const migrateTasks = function(ast) { const transform = transformations[key]; return { title: key, - task: () => new Promise((resolve, reject) => { - console.log(key); - try { - const res = (transform(jscodeshift, ast))(); - // setTimeout(resolve, 1000) - resolve(res); - } catch (err) { - reject(err); - } - }) + task: () => transform(ast) }; }); }; @@ -62,10 +53,14 @@ module.exports = function tranformFile(currentConfigPath, outputConfigPath, opti if (answers['confirmMigration']) { // TODO validate the config fs.writeFileSync(outputConfigPath, outputConfig, 'utf8'); - process.stdout.write(chalk.green(`Congratulations! Your new webpack v2 config file is at ${outputConfigPath}`)); + process.stdout.write(chalk.green(`✔︎ New webpack v2 config file is at ${outputConfigPath}`)); } else { - process.stdout.write(chalk.yellow('Migration aborted')); + process.stdout.write(chalk.red('✖ Migration aborted')); } }); + }) + .catch(err => { + console.error(chalk.red('✖ ︎Migration aborted due to some errors')); + console.error(err); }); }; diff --git a/lib/transformations/defineTest.js b/lib/transformations/defineTest.js index 64a2a77301d..8331b7469c2 100644 --- a/lib/transformations/defineTest.js +++ b/lib/transformations/defineTest.js @@ -33,7 +33,7 @@ const path = require('path'); * - Test data should be located in a directory called __testfixtures__ * alongside the transform and __tests__ directory. */ -function runTest(dirName, transformName, testFilePrefix) { +function runSingleTansform(dirName, transformName, testFilePrefix) { if (!testFilePrefix) { testFilePrefix = transformName; } @@ -53,8 +53,7 @@ function runTest(dirName, transformName, testFilePrefix) { jscodeshift = jscodeshift.withParser(module.parser); } const ast = jscodeshift(source); - const output = transform(jscodeshift, ast).toSource({ quote: 'single' }); - expect(output).toMatchSnapshot(); + return transform(jscodeshift, ast).toSource({ quote: 'single' }); } /** @@ -67,7 +66,8 @@ function defineTest(dirName, transformName, testFilePrefix) { : 'transforms correctly'; describe(transformName, () => { it(testName, () => { - runTest(dirName, transformName, testFilePrefix); + const output = runSingleTansform(dirName, transformName, testFilePrefix); + expect(output).toMatchSnapshot(); }); }); } diff --git a/lib/transformations/index.js b/lib/transformations/index.js index b16e05ef4af..c459c4c15b0 100644 --- a/lib/transformations/index.js +++ b/lib/transformations/index.js @@ -1,4 +1,5 @@ const jscodeshift = require('jscodeshift'); +const pEachSeries = require('p-each-series'); const loadersTransform = require('./loaders/loaders'); const resolveTransform = require('./resolve/resolve'); @@ -9,7 +10,7 @@ const bannerPluginTransform = require('./bannerPlugin/bannerPlugin'); const extractTextPluginTransform = require('./extractTextPlugin/extractTextPlugin'); const removeDeprecatedPluginsTransform = require('./removeDeprecatedPlugins/removeDeprecatedPlugins'); -const transformations = { +const transformsObject = { loadersTransform, resolveTransform, removeJsonLoaderTransform, @@ -20,29 +21,47 @@ const transformations = { removeDeprecatedPluginsTransform }; +const transformations = Object.keys(transformsObject).reduce((res, key) => { + res[key] = (ast) => transformSingleAST(ast, transformsObject[key]); + return res; +}, {}); + function transformSingleAST(ast, transformFunction) { - return transformFunction(jscodeshift, ast); + return new Promise((resolve, reject) => { + setTimeout(() => { + try { + resolve(transformFunction(jscodeshift, ast)); + } catch (err) { + reject(err); + } + }, 0); + }); } /* -* @function transform -* -* Tranforms a given source code by applying selected transformations to the AST -* -* @param { String } source - Source file contents -* @param { Array } transformations - List of trnasformation functions in defined the -* order to apply. By default all defined transfomations. -* @param { Object } options - Reacst formatting options -* @returns { String } Transformed source code -* */ + * @function transform + * + * Tranforms a given source code by applying selected transformations to the AST + * + * @param { String } source - Source file contents + * @param { Array } transformations - List of trnasformation functions in defined the + * order to apply. By default all defined transfomations. + * @param { Object } options - Reacst formatting options + * @returns { String } Transformed source code + * */ function transform(source, transforms, options) { const ast = jscodeshift(source); const recastOptions = Object.assign({ quote: 'single' }, options); transforms = transforms || Object.keys(transformations).map(k => transformations[k]); - transforms.forEach(f => transformSingleAST(ast, f)); - return ast.toSource(recastOptions); + return pEachSeries(transforms, f => f(ast)) + .then(() => { + return ast.toSource(recastOptions); + }) + .catch(err => { + console.error(err); + }); } module.exports = { diff --git a/lib/transformations/index.test.js b/lib/transformations/index.test.js index 6058db357b7..4953beb8640 100644 --- a/lib/transformations/index.test.js +++ b/lib/transformations/index.test.js @@ -31,26 +31,34 @@ module.exports = { `; describe('transform', () => { - it('should not transform if no transformations defined', () => { - const output = transform(input, []); - expect(output).toEqual(input); + it('should not transform if no transformations defined', (done) => { + transform(input, []).then(output => { + expect(output).toEqual(input); + done(); + }); }); - it('should transform using all transformations', () => { - const output = transform(input); - expect(output).toMatchSnapshot(); + it('should transform using all transformations', (done) => { + transform(input).then(output => { + expect(output).toMatchSnapshot(); + done(); + }); }); - it('should transform only using specified transformations', () => { - const output = transform(input, [transformations.loadersTransform]); - expect(output).toMatchSnapshot(); + it('should transform only using specified transformations', (done) => { + transform(input, [transformations.loadersTransform]).then(output => { + expect(output).toMatchSnapshot(); + done(); + }); }); - it('should respect recast options', () => { - const output = transform(input, undefined, { + it('should respect recast options', (done) => { + transform(input, undefined, { quote: 'double', trailingComma: true + }).then(output => { + expect(output).toMatchSnapshot(); + done(); }); - expect(output).toMatchSnapshot(); }); }); diff --git a/package.json b/package.json index c477aa7b661..771d3cd2059 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ "listr": "^0.11.0", "loader-utils": "^0.2.16", "lodash": "^4.17.4", + "p-each-series": "^1.0.0", "recast": "git://github.com/kalcifer/recast.git#bug/allowbreak", "rx": "^4.1.0", "supports-color": "^3.1.2", From bf0427e7da38d184ee7fbe5ce3e7bc00dbc3f684 Mon Sep 17 00:00:00 2001 From: Andrey Okonetchnikov Date: Thu, 23 Mar 2017 16:33:09 +0100 Subject: [PATCH 4/5] fix: Improve startup time by requiring code in appropriate branches --- bin/webpack.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/bin/webpack.js b/bin/webpack.js index eddebcb3998..2d3b4dde3d0 100755 --- a/bin/webpack.js +++ b/bin/webpack.js @@ -37,7 +37,6 @@ var yargs = require('yargs') require('./config-yargs')(yargs); - var DISPLAY_GROUP = 'Stats options:'; var BASIC_GROUP = 'Basic options:'; @@ -156,19 +155,15 @@ if(argv.verbose) { argv['display-cached-assets'] = true; } -var processOptions = require('./process-options'); -var initInquirer = require('../lib/initialize'); - if(argv.init) { - initInquirer(argv._); + require('../lib/initialize')(argv._); } else if(argv.migrate) { const filePaths = argv._; if (!filePaths.length) { throw new Error('Please specify a path to your webpack config'); } const inputConfigPath = path.resolve(process.cwd(), filePaths[0]); - require('../lib/migrate.js')(inputConfigPath, inputConfigPath); } else { - processOptions(yargs,argv); + require('./process-options')(yargs,argv); } From 55664e40e233e9f5e31cda1d1b6f41e9742fba14 Mon Sep 17 00:00:00 2001 From: Andrey Okonetchnikov Date: Thu, 23 Mar 2017 16:34:04 +0100 Subject: [PATCH 5/5] Better Listr usage and speed improvements --- lib/migrate.js | 67 ++++++++++++++++++++++-------------- lib/transformations/index.js | 9 ++--- package.json | 1 + 3 files changed, 48 insertions(+), 29 deletions(-) diff --git a/lib/migrate.js b/lib/migrate.js index d5829de7b97..059d529c309 100644 --- a/lib/migrate.js +++ b/lib/migrate.js @@ -1,39 +1,55 @@ const fs = require('fs'); -const diff = require('diff'); const chalk = require('chalk'); -const jscodeshift = require('jscodeshift'); -const transformations = require('./transformations').transformations; +const diff = require('diff'); const inquirer = require('inquirer'); +const PLazy = require('p-lazy'); const Listr = require('listr'); -const migrateTasks = function(ast) { - return Object.keys(transformations).map(key => { - const transform = transformations[key]; - return { - title: key, - task: () => transform(ast) - }; - }); -}; - -module.exports = function tranformFile(currentConfigPath, outputConfigPath, options) { +module.exports = function transformFile(currentConfigPath, outputConfigPath, options) { const recastOptions = Object.assign({ quote: 'single' }, options); - let currentConfig = fs.readFileSync(currentConfigPath, 'utf8'); - const ast = jscodeshift(currentConfig); const tasks = new Listr([ + { + title: 'Reading webpack config', + task: (ctx) => new PLazy((resolve, reject) => { + fs.readFile(currentConfigPath, 'utf8', (err, content) => { + if (err) { + reject(err); + } + try { + console.time('Reading webpack config'); + const jscodeshift = require('jscodeshift'); + ctx.source = content; + ctx.ast = jscodeshift(content); + resolve(); + console.timeEnd('Reading webpack config'); + } catch (err) { + reject('Error generating AST', err); + } + }); + }) + }, { title: 'Migrating config from v1 to v2', - task: () => new Listr(migrateTasks(ast)) + task: (ctx) => { + const transformations = require('./transformations').transformations; + return new Listr(Object.keys(transformations).map(key => { + const transform = transformations[key]; + return { + title: key, + task: () => transform(ctx.ast, ctx.source) + }; + })); + } } ]); tasks.run() - .then(() => { - const outputConfig = ast.toSource(recastOptions); - const diffOutput = diff.diffLines(currentConfig, outputConfig); - diffOutput.map(diffLine => { + .then((ctx) => { + const result = ctx.ast.toSource(recastOptions); + const diffOutput = diff.diffLines(ctx.source, result); + diffOutput.forEach(diffLine => { if (diffLine.added) { process.stdout.write(chalk.green(`+ ${diffLine.value}`)); } else if (diffLine.removed) { @@ -52,15 +68,16 @@ module.exports = function tranformFile(currentConfigPath, outputConfigPath, opti .then(answers => { if (answers['confirmMigration']) { // TODO validate the config - fs.writeFileSync(outputConfigPath, outputConfig, 'utf8'); - process.stdout.write(chalk.green(`✔︎ New webpack v2 config file is at ${outputConfigPath}`)); + fs.writeFileSync(outputConfigPath, result, 'utf8'); + console.log(chalk.green(`✔︎ New webpack v2 config file is at ${outputConfigPath}`)); } else { - process.stdout.write(chalk.red('✖ Migration aborted')); + console.log(chalk.red('✖ Migration aborted')); } }); }) .catch(err => { - console.error(chalk.red('✖ ︎Migration aborted due to some errors')); + console.log(chalk.red('✖ ︎Migration aborted due to some errors')); console.error(err); + process.exitCode = 1; }); }; diff --git a/lib/transformations/index.js b/lib/transformations/index.js index d5c37b8a518..721b66612f5 100644 --- a/lib/transformations/index.js +++ b/lib/transformations/index.js @@ -1,5 +1,6 @@ const jscodeshift = require('jscodeshift'); const pEachSeries = require('p-each-series'); +const PLazy = require('p-lazy'); const loadersTransform = require('./loaders/loaders'); const resolveTransform = require('./resolve/resolve'); @@ -22,15 +23,15 @@ const transformsObject = { }; const transformations = Object.keys(transformsObject).reduce((res, key) => { - res[key] = (ast) => transformSingleAST(ast, transformsObject[key]); + res[key] = (ast, source) => transformSingleAST(ast, source, transformsObject[key]); return res; }, {}); -function transformSingleAST(ast, transformFunction) { - return new Promise((resolve, reject) => { +function transformSingleAST(ast, source, transformFunction) { + return new PLazy((resolve, reject) => { setTimeout(() => { try { - resolve(transformFunction(jscodeshift, ast)); + resolve(transformFunction(jscodeshift, ast, source)); } catch (err) { reject(err); } diff --git a/package.json b/package.json index 66e16b912b0..915b52b4632 100644 --- a/package.json +++ b/package.json @@ -42,6 +42,7 @@ "loader-utils": "^0.2.16", "lodash": "^4.17.4", "p-each-series": "^1.0.0", + "p-lazy": "^1.0.0", "recast": "git://github.com/kalcifer/recast.git#bug/allowbreak", "rx": "^4.1.0", "supports-color": "^3.1.2",