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 passing --no-warnings on to node process #275

Closed
wants to merge 1 commit into from

Conversation

lehni
Copy link
Contributor

@lehni lehni commented Mar 20, 2022

to hide warnings about experimental-loader / experimental-modules

@bjornstar
Copy link
Collaborator

Could you add this to the list of known node command-line arguments?

I'd prefer to pass this on rather than hiding warnings when people aren't expecting that.

@lehni
Copy link
Contributor Author

lehni commented Mar 21, 2022

That makes sense. I tried to pass it first but I didn't make it through. And I was also under the impression that it needs to come before the loader argument, or it won't be effective there, but maybe I'm wrong. I'll spend more time on this tomorrow.

Useful e.g. to hide warnings about experimental-loader / experimental-modules
@lehni lehni changed the title [ESM] Pass --no-warnings to node process Allow passing --no-warnings on to node process Mar 22, 2022
@lehni
Copy link
Contributor Author

lehni commented Mar 22, 2022

@bjornstar I've adjusted this now. I needed to change fromunshift() to push() for the --experimental / --loader args, because they need to come after --no-warnings to have an effect.

@bjornstar
Copy link
Collaborator

Looks good, can I trouble you for a test to ensure the --no-warnings takes effect?

@lehni
Copy link
Contributor Author

lehni commented Mar 22, 2022

Argh, this is actually not working yet. I spent a while trying to understand what's wrong, and there are two problems:

  • minimist() turns --no-warnings into warnings: false and this does not work with the rest of the arguments handling that I am currently having a hard time to make sense out of (things like nodeArgsReducer(), unknownFactory(), it's all quite obscure to me).

  • unshift() was needed because these arguments need to come before the scripts!

@lehni
Copy link
Contributor Author

lehni commented Mar 22, 2022

The part that turns everything upside down is here:

https://github.com/substack/minimist/blob/7efb22a518b53b06f5b02a1038a88bd6290c2846/index.js#L118-L120

Sadly this behavior can't be turned off.

@bjornstar
Copy link
Collaborator

Argh, this is actually not working yet. I spent a while trying to understand what's wrong, and there are two problems:

  • minimist() turns --no-warnings into warnings: false and this does not work with the rest of the arguments handling that I am currently having a hard time to make sense out of (things like nodeArgsReducer(), unknownFactory(), it's all quite obscure to me).
  • unshift() was needed because these arguments need to come before the scripts!

node does a lot of custom argument parsing so it's quite challenging to replicate their behavior. We use nodeOptional to pull the known node arguments off of argv and handle them ourselves instead of passing them to minimist. Originally it was because they have optional values which minimist does not support hence the name optional. We can change the name to something better.

@bjornstar bjornstar mentioned this pull request Mar 22, 2022
@bjornstar
Copy link
Collaborator

I went ahead and implemented it, i'll get a release out shortly. Thanks for the contributions!

@bjornstar bjornstar closed this Mar 22, 2022
@lehni
Copy link
Contributor Author

lehni commented Mar 22, 2022

Happy to help where I can! Thank you for the fast responses!

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

Successfully merging this pull request may close these issues.

None yet

2 participants