-
-
Notifications
You must be signed in to change notification settings - Fork 602
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
Refactor lib/creator and integrate validation in migrate #99
Changes from 5 commits
77dc227
9a84b36
8870ec0
3dee6de
bd98877
717b61e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,42 +1,46 @@ | ||
const validateSchema = require('./utils/validateSchema.js'); | ||
const webpackOptionsSchema = require('./utils/webpackOptionsSchema.json'); | ||
const WebpackOptionsValidationError = require('./utils/WebpackOptionsValidationError'); | ||
const initTransform = require('./init-transform'); | ||
const chalk = require('chalk'); | ||
const yeoman = require('yeoman-environment'); | ||
const path = require('path'); | ||
const defaultGenerator = require('./yeoman/webpack-generator'); | ||
const WebpackAdapter = require('./yeoman/webpack-adapter'); | ||
const runTransform = require('./transformations/index'); | ||
|
||
/* | ||
* @function creator | ||
* | ||
* Main function to build up a webpack configuration. | ||
* Either throws an error if it doesn't match the webpack schema, | ||
* or validates the filepaths of the options given. | ||
* If a package is supplied, it finds the path of the package and runs inquirer | ||
* Runs yeoman and runs the transformations based on the object | ||
* built up from an author/user | ||
* | ||
* @param { Array } pkgPaths - An Array of packages to run | ||
* @param { <object|null> } opts - An object containing webpackOptions or nothing | ||
* @returns { <Function|Error> } initTransform - Initializes the scaffold in yeoman | ||
* @param { String } options - An path to the given generator | ||
* @returns { Function } runTransform - Run transformations based on yeoman prompt | ||
*/ | ||
|
||
module.exports = function creator(pkgPaths, opts) { | ||
// null, config -> without package | ||
// addon, null -> with package | ||
// we're dealing with init, we need to change this later, as it may have been emptied by yeoman | ||
if(!pkgPaths && !opts) { | ||
initTransform(); | ||
} | ||
else if(pkgPaths) { | ||
// this example app actually needs a refactor in order for it to work | ||
initTransform(pkgPaths); | ||
} | ||
else if(!pkgPaths && opts) { | ||
console.log(opts); | ||
// scaffold is done | ||
/* | ||
const webpackOptionsValidationErrors = validateSchema(webpackOptionsSchema, initialWebpackConfig); | ||
if (webpackOptionsValidationErrors.length) { | ||
throw new WebpackOptionsValidationError(webpackOptionsValidationErrors); | ||
} else { | ||
process.stdout.write('\n' + chalk.green('Congratulations! Your new webpack config file is created!') + '\n'); | ||
} | ||
*/ | ||
module.exports = function creator(options) { | ||
const env = yeoman.createEnv(null, null, new WebpackAdapter()); | ||
const generatorName = options ? replaceGeneratorName(path.basename(options)) : 'webpack-default-generator'; | ||
if(options) { | ||
const generatorName = replaceGeneratorName(path.basename(options)); | ||
env.register(require.resolve(options), generatorName); | ||
} else { | ||
env.registerStub(defaultGenerator, 'webpack-default-generator'); | ||
} | ||
|
||
env.run(generatorName) | ||
.on('end', () => { | ||
return runTransform(env.getArgument('configuration')); | ||
}); | ||
}; | ||
|
||
/* | ||
* @function replaceGeneratorName | ||
* | ||
* Replaces the webpack-addons pattern with the end of the addons name merged | ||
* with 'generator' | ||
* | ||
* @param { String } name - name of the generator | ||
* @returns { String } name - replaced pattern of the name | ||
*/ | ||
|
||
function replaceGeneratorName(name) { | ||
return name.replace( | ||
/(webpack-addons)?([^:]+)(:.*)?/g, 'generator$2'); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
'use strict'; | ||
|
||
function replaceGeneratorName(name) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be the real implementation? Export it and import here to remove code duplication, please. |
||
return name.replace( | ||
/(webpack-addons)?([^:]+)(:.*)?/g, 'generator$2'); | ||
} | ||
|
||
describe('replaceGeneratorName', () => { | ||
|
||
it('should replace a pattern of an addon', () => { | ||
const generatorName = replaceGeneratorName('webpack-addons-thefox'); | ||
expect(generatorName).toEqual('generator-thefox'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The regex looks more complex as just this test and covers more edge cases. Should we test against them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just an initial commit, can add more tests later and label an issue with |
||
}); | ||
|
||
}); |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
//const chalk = require('chalk'); | ||
//const validateSchema = require('../../utils/validateSchema.js'); | ||
//const webpackOptionsSchema = require('../../utils/webpackOptionsSchema.json'); | ||
//const WebpackOptionsValidationError = require('../../utils/WebpackOptionsValidationError'); | ||
/* | ||
* @function runTransform | ||
* | ||
* Runs the transformations from an object we get from yeoman | ||
* | ||
* @param { Object } transformObject - Options to transform | ||
* @returns { <Void> } TODO | ||
*/ | ||
|
||
module.exports = function runTransform(transformObject) { | ||
|
||
// scaffold is done | ||
/* | ||
const webpackOptionsValidationErrors = validateSchema(webpackOptionsSchema, initialWebpackConfig); | ||
if (webpackOptionsValidationErrors.length) { | ||
throw new WebpackOptionsValidationError(webpackOptionsValidationErrors); | ||
} else { | ||
process.stdout.write('\n' + chalk.green('Congratulations! Your new webpack config file is created!') + '\n'); | ||
} | ||
*/ | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,9 @@ const diff = require('diff'); | |
const inquirer = require('inquirer'); | ||
const PLazy = require('p-lazy'); | ||
const Listr = require('listr'); | ||
const validateSchema = require('./utils/validateSchema.js'); | ||
const webpackOptionsSchema = require('./utils/webpackOptionsSchema.json'); | ||
const WebpackOptionsValidationError = require('./utils/WebpackOptionsValidationError'); | ||
|
||
module.exports = function transformFile(currentConfigPath, outputConfigPath, options) { | ||
const recastOptions = Object.assign({ | ||
|
@@ -65,9 +68,17 @@ module.exports = function transformFile(currentConfigPath, outputConfigPath, opt | |
]) | ||
.then(answers => { | ||
if (answers['confirmMigration']) { | ||
// TODO validate the config | ||
fs.writeFileSync(outputConfigPath, result, 'utf8'); | ||
console.log(chalk.green(`✔︎ New webpack v2 config file is at ${outputConfigPath}`)); | ||
fs.writeFile(outputConfigPath, result, 'utf8', (err) => { | ||
const webpackOptionsValidationErrors = validateSchema(webpackOptionsSchema, require(outputConfigPath)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this go to a separate PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not needed. Fixing the transform issues and let's merge and take it from there. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm working on a fix PR and desperately need this! :) |
||
if (err) { | ||
throw err; | ||
} | ||
else if (webpackOptionsValidationErrors.length) { | ||
throw new WebpackOptionsValidationError(webpackOptionsValidationErrors); | ||
} else { | ||
console.log(chalk.green(`\n ✔︎ New webpack v2 config file is at ${outputConfigPath}`)); | ||
} | ||
}); | ||
} else { | ||
console.log(chalk.red('✖ Migration aborted')); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind adding tests for this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, giveme 1h :) At school