search

microsoft / types-publisher

This repo has moved:
https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/publisher
MIT License
388 stars 151 forks source link

fix: add @types/mkdirp-promise to whitelist #736

Closed JustinBeckwith closed 4 years ago

JustinBeckwith commented 4 years ago

disclaimer: I have no idea what I'm doing

Greeting folks!
Recently, the npm module mkdirp when through a rewrite. The interface was changed from being callback based to promise based.

DefinitelyTyped includes an @types/mkdirp package, which is currently out of date as compared to the new shiny 1.0 of mkdirp. It snaps to the 0.5.1 version of the module.

I would like to update the types to support 1.0! So I submitted this shiny PR: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/42108

The change to @types/mkdirp worked great, but ... tests for @types/mkdirp-promise started to fail. It turns out the promise variant of the module was depending on the local callback based types.

I tried to figure out from the README how to handle this, and ended up trying to add a package.json to my PR so I could specify the supported versions of @types/mkdirp to use along with @types/mkdirp-promise. The directions from the README of DefinitelyTyped led me here, to submit this PR.

I can't help but feel like I'm doing something wrong, since I get the gist that this whitelist is about modules that used to have DefinitelyTyped packages, and then started doing their own distributions. I am just a simple human trying to use mkdirp@1.0.0 :)

Any help, feedback, or guidance y'all could provide would be greatly appreciated!

sandersn commented 4 years ago

The two questions to answer are:

  1. Does mkdirp-promise depend on mkdirp 1.0? (Or, since mkdirp 1.0 is only 12 days old, do the authors plan to update it?)
  2. If so, then the correct thing is to update mkdirp-promise too. If not, then this PR is the correct fix.
sandersn commented 4 years ago

1a: No, it depends on mkdirp: "*", which is a bit :trollface: (https://github.com/ahmadnassri/mkdirp-promise/pull/88 fixes this) 1b: @AArnott's comment indicates that mkdirp-promise is obsolete since mkdirp 1.0, meaning that it will never depend on 1.0. (Although Andrew is not the owner of the project, I know from experience that he is fearsomely knowledgeable.) 2: Therefore, this PR is the correct fix.

I'll go ahead and merge it and let you know when CI is updated.

sandersn commented 4 years ago

Actually, come to think of it, another solution is to delete types/mkdirp-promise in the DT repo. This stops publication of the package without deprecating it. Few enough people use @types/mkdirp-promise that I don't think it will need updates in the future.

@JustinBeckwith It's up to you which way you want to go. CI should have the newest version of types-publisher now.