Skip to content
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

Allow to pass flags to node.js #1195

Merged
merged 6 commits into from
Jan 30, 2020
Merged

Conversation

smelukov
Copy link
Member

What kind of change does this PR introduce?

feature

Did you add tests for your changes?

yes

If relevant, did you update the documentation?

About --node-args option

Summary

closes #1084
closes #289

Does this PR introduce a breaking change?

no

Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some comments. Nice PR! Also, would you mind to add some tests inside the test folder?

lib/bootstrap.js Show resolved Hide resolved

if (helpFlagExists) {
cli.runHelp(process.argv);
return;
} else if (versionFlagExists) {
cli.runVersion();
return;
} else if (nodeArgsExists) {
args = cmdArgs(core, { stopAtFirstUnknown: false, partial: true });
const [, , ...rawArgs] = process.argv;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gosh this is lovely! <3

lib/bootstrap.js Outdated Show resolved Hide resolved
lib/bootstrap.js Outdated Show resolved Hide resolved
lib/bootstrap.js Outdated Show resolved Hide resolved
@webpack-bot
Copy link

@smelukov Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@ematipico Please review the new changes.

@ghost
Copy link

ghost commented Jan 29, 2020

There were the following issues with this Pull Request

  • Commit: c91b06f
    • ✖ body must have leading blank line

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one last thing, I think. In case we pass a arguement that is not recognised by node and node throws an error, how do we handle that error? I believe we should show a correct error, saying to the user that "too" argument is not a node argument. Basically WHY the CLI blown up.

@smelukov
Copy link
Member Author

smelukov commented Jan 30, 2020

I believe we should show a correct error, saying to the user that "too" argument is not a node argument

@ematipico It already works

> node cli.js --node-args "-foo=123"
node: bad option: -foo=123

@webpack-bot
Copy link

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@rishabh3112 rishabh3112 merged commit da08bb1 into webpack:next Jan 30, 2020
@rishabh3112
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants