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

Dpetev/yargs bump 10.1.x #1096

Closed
wants to merge 6 commits into from
Closed

Conversation

damyanpetev
Copy link
Member

Closes # .

Additional information related to this pull request:

@damyanpetev damyanpetev changed the base branch from master to 10.1.x January 26, 2023 17:47
@coveralls
Copy link

coveralls commented Jan 26, 2023

Coverage Status

coverage: 70.141% (-0.02%) from 70.158%
when pulling 884d841 on dpetev/yargs-bump-10.1.x
into 6e8b984 on 10.1.x.

@damyanpetev
Copy link
Member Author

@PlamenaMiteva @Lipata
The changes in the PR are for the most part bumping yargs to latest, though I'm assuming there are yarn.lock diffs in latest and it might not be feasible to reuse this PR and might need to be recreated. If not, this could also be retargeted just as easily.

If useful here are the steps and some issues I found along the way that need to be resolved.

yarn workspace igniteui-cli add yargs@latest

And since we didn't have those, I figured types would be nice and help with the migration process, so also

yarn workspace igniteui-cli add @types/yargs@latest --dev

And they sort of did, the notable change being that .parseSync() is now needed to go back to the old behavior we used. However, there's an issue with the options we pass to .command(), because our objects contain extra stuff and thus the multiple as any type asserts in the PR. I believe those confuse the overload matching and it ends up going for a different version, though the type errors might be perfectly valid too - needs to be checked against typing and docs and adjusted.
PS: If we update TypeScript satisfies looks nice :)

One more thing I landed on - the typescript-json-schema we use for the config schema failed trying to parse the new yargs type (which is why I reverted those to see if the build will work). Tried updating, though that didn't help - might be reliant on the typescript version itself so update there might be needed. There's also the question of why was the schema trying to parse the yargs types in the first place - there's a good chance it's better to run it with its own config which is possible looking at the docs, cuz I'm pretty sure it doesn't need to use those types to produce the schema for a few of our interfaces.

Lastly, not sure if it's just on my end, but I can't run tests locally due to lint fails which mysteriously don't happen on the build machines. @Lipata this might be a separate task/issue, but there's something wrong with the lint setup, ignoring the fact that tslint should be replaced with eslint too.

@github-actions
Copy link

github-actions bot commented Apr 8, 2023

There has been no recent activity and this PR has been marked inactive.

@github-actions
Copy link

github-actions bot commented Jun 9, 2023

There has been no recent activity and this PR has been marked inactive.

@github-actions
Copy link

github-actions bot commented Sep 3, 2023

There has been no recent activity and this PR has been marked inactive.

Copy link

github-actions bot commented Nov 3, 2023

There has been no recent activity and this PR has been marked inactive.

Copy link

github-actions bot commented Jan 4, 2024

There has been no recent activity and this PR has been marked inactive.

Copy link

There has been no recent activity and this PR has been marked inactive.

@github-actions github-actions bot closed this Mar 22, 2024
@damyanpetev damyanpetev deleted the dpetev/yargs-bump-10.1.x branch September 2, 2024 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants