-
-
Notifications
You must be signed in to change notification settings - Fork 199
Migrate from yargs to yargs-parser for the CLI #505
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
Conversation
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.
There is another require('yargs/yargs')
in configureRuntimeEnvironment()
, which is why that job fails.
We are probably missing a test case for that one since only the linter detected it.
Hm... actually the method is called for all the tests of |
I don't think we can do something about it. |
The encore CLI is not implemented using the yargs features, as it is a wrapper around the webpack and webpack-dev-server. Only the parsing from yargs was used. And this is implemented by yargs-parser. Depending on that package directly avoids bring the full (outdated) yargs.
I fixed the place I missed. Regarding tests, there is indeed nothing we can do here, due to the way yarn and npm are flattening the Yarn's PnP mode may allow detecting such things (as it builds |
And all these were requiring different versions of yargs, so we were not sharing it with them previously. |
Note that when using snyk.io on my project, it reports it being affected by https://app.snyk.io/vuln/npm:mem:20180117 due to 2 things:
This PR solves this |
Not a big issue in this case but it's always nice to remove that kind of warning. |
yeah, I know it is not a big issue (otherwise, I would have sent the PR weeks ago when I saw it for the first time) |
The encore CLI is not implemented using the yargs features, as it is a wrapper around the webpack and webpack-dev-server. Only the parsing from yargs was used. And this is implemented by yargs-parser. Depending on that package directly avoids bring the full (outdated) yargs.