-
-
Notifications
You must be signed in to change notification settings - Fork 603
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
Webpack-CLI version 1 #105
Conversation
lib/creator/transformations/index.js
Outdated
const transformsObject = { | ||
entryTransform, | ||
outputTransform | ||
}; |
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.
This should be passed to runTransform
somehow.
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.
Doing through p-lazy
I suppose ;)
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.
Don't review yet, will notify when there's something better to review, this isn't nearly done at all
if(!webpackProperties['output']) { | ||
return ast; | ||
} else if(webpackProperties['output'].length) { | ||
throw new Error('Supplying output with only no options is not supported.'); |
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.
Remind me to change this l8tr :)
Awesome work @ev1stensberg |
Thanks to Sindre Sørhus for this fix. Object references are now making sure our callbacks are held, and removed the return of the promise and rather used the object we want to return.
lib/creator/yeoman/utils/entry.js
Outdated
@@ -6,44 +6,32 @@ module.exports = (self, answer) => { | |||
|
|||
let entryIdentifiers; | |||
let entryProperties; | |||
if(answer['entryType'].indexOf('Array') >= 0) { | |||
if(answer['entryType'] == 'Multiple') { |
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.
==
-> ===
lib/creator/yeoman/utils/entry.js
Outdated
} | ||
resolve(webpackEntryPoint); | ||
}); | ||
}); | ||
} | ||
else { | ||
self.prompt([ | ||
Input('singularEntry', 'What\'s your entry point?') | ||
Input('singularEntry', 'Write the location of module you would like to use') |
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.
I'm not sure write
is a good verb here. I'd say type
?
this.configuration.config.webpackOptions.module = getDefaultLoaders(); | ||
this.configuration.config.webpackOptions.plugins = getDefaultPlugins(); | ||
this.prompt([ | ||
RawList('entryType', 'What kind of entry point do you want?', ['Array', 'Function', 'String', 'Object']) | ||
RawList('entryType', 'Would you like to bundle a single or multiple javascript modules?', |
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.
Please put each argument on a new line
Hey, @ev1stensberg - I checked out the latest and ran
But, I would suggest an alternate set of questions to ask. The script would be something like Welcome to webpack!
We will ask you a few questions to create your configuration file.
1. Enter the name of your entry file - index.js
2. Enter the path for the output file - dist/bundle.js
3. Do you use ES6 in your project? Y/n
4. Do you use one of the below CSS solutions?
a. SASS
b. LESS
c. PostCss
5. Do you want to bundle the CSS to a separate file? Y/n
Creating "webpack.config.js"
module.exports = {
...
}
Do you want to save this file and install dependencies? Y/n
Installing dependencies....
..
..
Congratulations! Your configuration file has been created at `webpack,config.js`
I feel we have the ast support to convert all the above questions to a configuration. This should suffice to be the default Plugin packages could extend these questions to include framework/library specific questions like preact/react etc. |
@pksjce Hi! Great to have you reviewing! Here's my replies to your concerns:
We can, however, try to implement the string addition and remove this burden from the users. Not guaranteeing it will work 100%, but I'll try implementing it along with file-extensions for single entries. I was thinking about consistency when adding the
For the CSS part, will refactor. Do you have a chance to review the ast part too? Edit: I'm not sure if beginners know what those are, we should stick with CSS only and ask for feedback from the community, perhaps? A question for me is: What is missing after the fixes mentioned to merge and push |
A small note to add is that Typing entry now lets you add things such as |
Fwiw, the proposal by @pksjce in "But, I would suggest an alternate set of questions to ask. The script would be something like" sounds like it would tweak the prompts to be a little more beginner friendly. Perhaps 'Do you use ES2015 in your project?' instead of ES6 :) Silly question: why wouldn't the user want to create their Webpack config in the last step of 0ba9d7d? It seems extraneous to me. At least with Yeoman generators, the UX we typically use is asking questions and on the final question then kicking of writing to disk. Reading through @ev1stensberg's work in c58f793, I do like the idea of minimal boilerplate for encouraging the use of common vendor chunks and ExtractTextPlugin. Another silly q: To what extent if any should we be switching over to babel-minify if the user opts for ES2015 support with Babel? Are we encouraging ES2015 -> ES5 -> Uglify that code? |
@pksjce @addyosmani Corrected the generator as of the comments. The last thing left is the CSS/SASS/LESS prompts, do we want those? I'm not sure, but open for input. Removed the confirm, even though I side with @pksjce on the confirm part, as this is consistent with Also, I think we should enable |
Also, heard some things about |
d24f50d
to
bc24b9e
Compare
Yeah, we did some tests couple of weeks back - babili wasn't terribly bigger (~10%), but it was way slower than babel+uglify.
This may have changed in the meantime though (babili is a beta project after all). |
cc @hzoo as an fyi on the babili part of the discussion 👍 for the current status of this work landing @ev1stensberg
👍 |
migrate.md file is empty. Is it intentional? |
Webpack CLI version 1
AST's
Prettier
Windows support
Documentation
Tests
Generator with adapter for us later to use for path validation before sending to AST's