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

proto-loader: Fix yargs types & update README #5

Conversation

badsyntax
Copy link

@badsyntax badsyntax commented Dec 20, 2020

There seems to have been a type update via patch version in @types/yargs where argv._ is now defined as _: Array<string | number>;, see:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/6de1fd533383aa8fcac9460edd2ee5d36989fa7b/types/yargs/index.d.ts#L643

This is why we need to do argv._ as string[] as well as add parse-positional-numbers: false parser config to prevent yargs from parsing positional args to numbers. I need to use @ts-ignore because the types for yargs-parser need updating, see: DefinitelyTyped/DefinitelyTyped#50196

I noticed this as a did a clean clone of this repo follow by a npm install and the compile script failed to build.

I also added example usage to the README as I believe this will help with understanding how to use the generator.

@badsyntax badsyntax force-pushed the proto-loader_type_generator--fix-yargs-types branch from c6762a3 to 1d8c2a6 Compare December 24, 2020 11:40
@badsyntax badsyntax marked this pull request as ready for review December 24, 2020 11:40
@badsyntax
Copy link
Author

@murgatroid99 this is now good for a review.

@murgatroid99
Copy link
Owner

Git sees this as conflicting with the previous PR you sent, so can you resolve that? Also, did you intend to update all of those dependencies? I'm uncertain about the major version bumps to gts and especiallly typescript.

@badsyntax badsyntax force-pushed the proto-loader_type_generator--fix-yargs-types branch from 1d8c2a6 to cf9d0fd Compare January 8, 2021 08:11
@badsyntax
Copy link
Author

I've simplified the changes.

Not sure why I bumbed gts and typescript but i've reverted those changes.

And there's no need to bumb any yargs package versions because the type fixes for yargs-parser are "automatically" pulled in due to the usage of the star (*) version number defined in the @types/yargs package:

Screenshot 2021-01-08 at 08 15 29

I'm sorry for force pushing, but it was easier this way due to the conflicts.

@murgatroid99 murgatroid99 merged commit 0496bcb into murgatroid99:proto-loader_type_generator Jan 8, 2021
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.

2 participants