Skip to content

Commit

Permalink
Fix: Only try to remove deprecated plugins from array expressions (#91)
Browse files Browse the repository at this point in the history
* fix: Guard new exepressions outside of arrays

For now only plugins instantiated in an array can be removed since this is safe.
All other usages are considered not safe and thus won't be modified.

- Display a message about the current support

Closes #83

* style: Better error messages

- Pass the source to the transform function
- This allows displaying code snippets in error messages

* fix: Use console.log to fix colors and use better error copy.

- Don't use `console.error` to fix colors.
- Use chalk instead to make output more prettier.
- Better error message with an example of the expected usage.
  • Loading branch information
okonet authored and Pavithra Kodmad committed Mar 21, 2017
1 parent 37a594d commit d060ad8
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 10 deletions.
2 changes: 1 addition & 1 deletion lib/transformations/defineTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ function runTest(dirName, transformName, testFilePrefix) {
jscodeshift = jscodeshift.withParser(module.parser);
}
const ast = jscodeshift(source);
const output = transform(jscodeshift, ast).toSource({ quote: 'single' });
const output = transform(jscodeshift, ast, source).toSource({ quote: 'single' });
expect(output).toMatchSnapshot();
}

Expand Down
2 changes: 1 addition & 1 deletion lib/transformations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ 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 => f(jscodeshift, ast, source));
return ast.toSource(recastOptions);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,14 @@ module.exports = {
}
"
`;

exports[`removeDeprecatedPlugins transforms correctly using "removeDeprecatedPlugins-3" data 1`] = `
"// This should throw
export default (config) => {
config.plugins.push(new webpack.optimize.UglifyJsPlugin());
config.plugins.push(new webpack.optimize.DedupePlugin());
config.plugins.push(new webpack.optimize.OccurrenceOrderPlugin());
return config
}
"
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// This should throw
export default (config) => {
config.plugins.push(new webpack.optimize.UglifyJsPlugin());
config.plugins.push(new webpack.optimize.DedupePlugin());
config.plugins.push(new webpack.optimize.OccurrenceOrderPlugin());
return config
}
Original file line number Diff line number Diff line change
@@ -1,21 +1,48 @@
const findPluginsByName = require('../utils').findPluginsByName;
const codeFrame = require('babel-code-frame');
const chalk = require('chalk');
const utils = require('../utils');

module.exports = function(j, ast) {
const example = `plugins: [
new webpack.optimize.OccurrenceOrderPlugin(),
new webpack.optimize.UglifyJsPlugin(),
new webpack.optimize.DedupePlugin()
]`;

module.exports = function(j, ast, source) {
// List of deprecated plugins to remove
// each item refers to webpack.optimize.[NAME] construct
const deprecatedPlugingsList = [
'webpack.optimize.OccurrenceOrderPlugin',
'webpack.optimize.DedupePlugin'
];

return findPluginsByName(j, ast, deprecatedPlugingsList)
return utils.findPluginsByName(j, ast, deprecatedPlugingsList)
.forEach(path => {
// Check how many plugins are defined and
// if there is only last plugin left remove `plugins: []` completely
if (path.parent.value.elements.length === 1) {
j(path.parent.parent).remove();
// For now we only support the case there plugins are defined in an Array
if (path.parent &&
path.parent.value &&
utils.isType(path.parent.value, 'ArrayExpression')
) {
// Check how many plugins are defined and
// if there is only last plugin left remove `plugins: []` node
if (path.parent.value.elements.length === 1) {
j(path.parent.parent).remove();
} else {
j(path).remove();
}
} else {
j(path).remove();
const startLoc = path.value.loc.start;
console.log(`
${ chalk.red('Only plugins instantiated in the array can be automatically removed i.e.:') }
${ codeFrame(example, null, null, { highlightCode: true }) }
${ chalk.red('but you use it like this:') }
${ codeFrame(source, startLoc.line, startLoc.column, { highlightCode: true }) }
${ chalk.red('Please remove deprecated plugins manually. ') }
See ${ chalk.underline('https://webpack.js.org/guides/migrating/')} for more information.`);
}
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ const defineTest = require('../defineTest');
defineTest(__dirname, 'removeDeprecatedPlugins', 'removeDeprecatedPlugins-0');
defineTest(__dirname, 'removeDeprecatedPlugins', 'removeDeprecatedPlugins-1');
defineTest(__dirname, 'removeDeprecatedPlugins', 'removeDeprecatedPlugins-2');
defineTest(__dirname, 'removeDeprecatedPlugins', 'removeDeprecatedPlugins-3');
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"test": "jest --coverage"
},
"dependencies": {
"babel-code-frame": "^6.22.0",
"babel-core": "^6.21.0",
"babel-preset-es2015": "^6.18.0",
"babel-preset-stage-3": "^6.17.0",
Expand Down

0 comments on commit d060ad8

Please sign in to comment.