(WIP) Cli node flags #439
(WIP) Cli node flags #439
Conversation
|
@playma256 The most important CI builds failed. This way your PR can't be merged. Please take a look at the CI results from travis (failure) and fix these issues. |
bin/webpack.js
Outdated
| Author Tobias Koppers @sokra | ||
| */ | ||
|
|
||
| (function() { |
There was a problem hiding this comment.
Don't remove IIFEE here, its for perf + being able to use return.
bin/webpack.js
Outdated
| // Prefer the local installation of webpack-cli | ||
| if (importLocal(__filename)) { | ||
| return; | ||
| const spawn = require("cross-spawn"); |
There was a problem hiding this comment.
Is everything from here taken from the previous PR? #377
|
@playma256 Thanks for your update. I labeled the Pull Request so reviewers will review it again. @ev1stensberg Please review the new changes. |
|
fixed somethings but appveyor seems to be breaking on something else... even when i rolled back these changes, any ideas? |
bin/process-node-options.js
Outdated
| }); | ||
|
|
||
| // terminate children | ||
| process.on("SIGINT", () => { |
There was a problem hiding this comment.
This & webpackCliProcess didn't throw as intended in the last PR. Make sure to exit the scope entirely.
| }); | ||
|
|
||
| // when main process was exited. | ||
| process.on("exit", (code) => { |
There was a problem hiding this comment.
This was the part "missing" @ev1stensberg , the other one above just relied on Ctrl/Cmd+C which sometimes was not the case. I've compared twice master branch and this branch, both are running almost identical (spawning 1 process, when tests for node-flags are running) in terms of process being spawned and this branch is not spawning billions anymore. I'm going to keep an eye on the CI to see if it fails, if so why.
|
It is failing, but it isnt even starting to run the tests, it is failing with the following error message: |
|
Which nodejs version are you on? Also, could you rebase against the |
|
I'm on 8.11 and also tested on 6.11. I'm going to double test on v6 before rebasing. |
|
Also, should this be merged into next too? |
|
Only next, it's a flaky PR, needs testing before releasing. Better to try out on next branch before publishing |
* cli(refactor): improve folder structure * chore(linting): fix linter errors * cli(filepath): use local import instead * cli(migrate): refactor error handling * chore(review): fix review comments * chore(review): fix review comments * chore(review): fix review comments
…tation (#379) * docs(commits): listed the list of type of commits available
* cli(init): mode support to config
* cli(init): skip redundant question * cli(init): use mini-css-extract-plugin
…392) * cli(refactor): fetch available modes directly from webpack options schema * cli(refactor): Retrieve information from webpackOptionSchema
* misc(add): variable parity, prettify * cli(add): write config to yeoman-rc * misc(add): improve generator questions
improved output filename
* cli(migration): Update UglifyJS migration file to fit webpack4 configuration * cli(migration): Add cases where require variable does not need to exist * cli(migration): Add transformation for usage of webpack.otimization.UglifyPlugin * cli(tests): Update test snapshots after updating transformation * cli(fix): fix expressionContent being null * cli(refactor): remove plugins array if empty Created function on ast-utils so every other transformation can use this. * tests: add tests for new ast-utils method * fix: fix test names and jsdoc * fix: update maxSize for utils folder
* ast(refactor): wip refactor * ast(refactor): wip refactor * ast(init): refactor * test(refactor): refactor test suite * tests(define): swap args * ast(parsing): refactor stuff * ast(init): refactor * ast(init): refactor tests * chore(tests): remove some unneeded tests * chore(pkg): update package.json * chore(project): clear up project structure * chore(cli): remove unneded files * chore(git): add gitignore to yeoman file * chore(deps): update pkg.json * tests(snapshots): update snapshots * tests(jest): use empty module for snapshots * tests(snap): only test one prop
* chore(release): [WIP] add semantic-release * test(ci): wip * test(ci): add node versions * test(ci): remove extra test * tests(ci): revise * tests(ci): only push to npm on master * tests(ci): use matrix on jobs * tests(ci): revise * tests(ci): update * tests(ci): test * tests(ci): test * tests(ci): p * tests(ci): update travis.yml
* use yargs.command instead of yargs.option for sub-commands * cli(node): Add node flags to CLI (#377) * feat: add support for node flags * tests: Fix node-flags test * misc: Fix test failing due to not-found webpack-cli * misc: remove comment * misc: refactor removing unecessary args * tests: add more tests to prevent argument collision * cli(cmds): remove strict * fix(node): remove node option for now
Adds fancy templates
* cli(init): revise installation steps * chore(formatting): format code * cli(tests): refactor tests
chore(greenkeeper): Update dependencies to enable Greenkeeper 🌴
|
@ev1stensberg it messed up everything... i'm going to start a branch from next and open a new pr... it will start fresh. Brb. |
What kind of change does this PR introduce?
Feature
Did you add tests for your changes?
Yes
If relevant, did you update the documentation?
Summary
The last implementation of allowing node flags into cli was buggy and after talking to @ev1stensberg he told me what was happening, something i did not realize in the past.
So, @ev1stensberg i've tested this with a real world project, and i did not have any bizillions of proccess being spawned. Please have a look, this time cis are working 100%.