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

chore(ts): ship yargs.d.ts #1671

Merged
merged 4 commits into from Jul 19, 2020
Merged

chore(ts): ship yargs.d.ts #1671

merged 4 commits into from Jul 19, 2020

Conversation

mleguen
Copy link
Member

@mleguen mleguen commented Jun 4, 2020

1st part of 5th step of #1586: this PR ships a yargs.d.ts adapted from the one in @types/yargs.

@bcoe
Copy link
Member

bcoe commented Jun 4, 2020

@mleguen I think in the spirit of SemVer, I would call this step breaking:

  • pulling in our own yargs.d.ts will mean that users can't easily override @types/yargs.
  • it sounded like there were a few changes to @types/yargs required.

Should we just call this our first commit to yargs@16.x, and land #1670 and make this the last patch we land in 15.4.0.


I think we should perhaps let 15.4.0 bake for a few days, after we release it, since it's our first significant TypeScript release.

but, after that short baking period, let's march towards the 16.x release.

@mleguen
Copy link
Member Author

mleguen commented Jun 20, 2020

Sorry, I did not progress much on this the past couple of weeks. I only had time to read the incoming notifications from github... I will try to do better in the coming weeks :-)

@bcoe
Copy link
Member

bcoe commented Jun 22, 2020

@mleguen how do you feel about me getting yargs@15.4.0 released, as I think our continued TypeScript work will be breaking ... perhaps we could try to land the .mjs import too, based on your experiments, and then plan that we'll probably be taking breaking changes to get this work over the finish line.

@mleguen
Copy link
Member Author

mleguen commented Jun 22, 2020 via email

@bcoe
Copy link
Member

bcoe commented Jul 1, 2020

@mleguen I did some testing, Node 14 actually seems pretty smart about hits handling of common JS. With yargs, as it is published today, this just works (I believe):

import yargs from 'yargs'
  
console.info(yargs.argv)

Without us needing to add any conditional exports. So, I don't think a ton of value in us adding the conditional exports in our case.

@bcoe bcoe marked this pull request as ready for review July 19, 2020 02:27
@bcoe bcoe merged commit c06f886 into master Jul 19, 2020
@bcoe bcoe deleted the 1586-yargs-d-ts branch July 19, 2020 02:32
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