chore(add-generator): changed the naming of the plugin in config file #893
Conversation
Changed the naming of the plugin to better and standard and bug free name in add-generator in plugin question
|
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
packages/generators/add-generator.ts
Outdated
| 0, | ||
| normalizePluginName.charAt(0).toUpperCase() | ||
| ); | ||
| let myPluginNameArray = answerToAction.actionAnswer.split("-") |
There was a problem hiding this comment.
Could you extract this new logic into a separate function and test it against few different plugins? There are the native ones too webpack.DefinePlugin.
There was a problem hiding this comment.
+1, this could quickly turn into a mess
There was a problem hiding this comment.
^ What I mean by that is that like @ematipico is saying, is that it might be hard to verify that this works unless we test it thoroughly. A better thing to do, would be to abstract the logic so we can run unit tests on it and make sure that it is working as expected.
There was a problem hiding this comment.
Okay, So Should I add it in utils and write tests for it. If it's fine? @evenstensberg @ematipico . Just the naming method
There was a problem hiding this comment.
Probably nice idea, or to add it within the generator itself if it's only used here
There was a problem hiding this comment.
@evenstensberg Yeah....I was talking about the utils folder inside the generators package. Will add it there with test then
There was a problem hiding this comment.
That's a great location. Go for it
|
Thanks @anikethsaha, |
|
@rishabh3112 true! |
|
@ematipico yeah I can do that. |
Added generatePluginName in generators/utils/plugins.ts and also moved the replaceAt method from add-generator.ts to generators/utils/plugins.ts. Added tests for the plugin name method
webpack-bot
added
PR: review-outdated
and removed
PR: reviewed-changes-requested
labels
|
cc @ematipico @evenstensberg , I made those changes. PTAL |
Added if stmt for not doing anything when native plugins comes-in Also Changed the test
fix the indentation
changed the loop
assigned length of the plugin to a constant
|
@evenstensberg Thanks for your update. I labeled the Pull Request so reviewers will review it again. @ematipico Please review the new changes. |
What kind of change does this PR introduce?
refactoring/bugfix
Did you add tests for your changes?
Yes
If relevant, did you update the documentation?
NA
Summary
Changed the naming of the current naming of the webpack plugin in the
add-generatorto a better and standard way of naming. The earlier one was creating error name when passed something likeextract-text-webpack-plugin, it would give the plugin in config file like thisBug
plugins: new Extract() - textPlugin,With this refractor
It will output the config file plugin with this
plugins: new ExtractTextWebpackPlugin(),Does this PR introduce a breaking change?
No
Other information
NA