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

update tmp type definitions to 0.1.0 #31

Closed
viceice opened this issue May 16, 2019 · 15 comments
Closed

update tmp type definitions to 0.1.0 #31

viceice opened this issue May 16, 2019 · 15 comments

Comments

@viceice
Copy link

viceice commented May 16, 2019

Please update the type definitions to @types/tmp 0.1.0, otherwise we will get wrong typings and missing Options and SimpleOptions.

@paulmelnikow
Copy link
Collaborator

Hi! That was fixed in #27 after 1.1.0 was released and shipped in 2.0.0. However 2.0.0 was unpublished due to a regression (see #29). Hopefully it will be released again soon!

@benjamingr
Copy link
Owner

This is now closed with 2.0.1 :]

@viceice
Copy link
Author

viceice commented May 21, 2019

Maybe we should add @types/tmp as a peerDependency?

@paulmelnikow
Copy link
Collaborator

Probably it should be a dependency.

@benjamingr
Copy link
Owner

Yeah that sounds like a good idea. PR welcome.

@viceice
Copy link
Author

viceice commented May 21, 2019

I wond add it as dependency, because it is only required if you use typescript. and you only would need it as a dev dependency

@benjamingr
Copy link
Owner

you only would need it as a dev dependency

Since tmp doesn't maintain its own types (right?) I think the reasonable thing to do would be to publish @types/tmp-promise that would rely on @types/tmp as a dependency and then people could install that as a devDependency.

@viceice
Copy link
Author

viceice commented May 21, 2019

the best would be to integrate the @types/tmp to tmp package

@benjamingr
Copy link
Owner

the best would be to integrate the @types/tmp to tmp package

@silkentrance what do you think?

@paulmelnikow
Copy link
Collaborator

paulmelnikow commented May 21, 2019

Since tmp doesn't maintain its own types (right?) I think the reasonable thing to do would be to publish @types/tmp-promise that would rely on @types/tmp as a dependency and then people could install that as a devDependency.

Hmm, that would mean we'd stop publishing the types ourselves. It is easier to keep the definitions up to date (as we're now type-checking them) if we maintain them in this repo and publish them as part of our own package. The @types namespace is maintained through DefinitelyTyped.

and you only would need it as a dev dependency

That's not true. If you use tmp-promise as a dev dependency, then you need @types/tmp as a dev dependency. If you use tmp-promise as a prod dependency, then @types/tmp also needs to be a prod dependency.

the best would be to integrate the @types/tmp to tmp package

That's a nice idea. I'd be happy to make a PR to do that.

@benjamingr
Copy link
Owner

then @types/tmp also needs to be a prod dependency.

Why? You mean if you're shipping typescript to production and compiling on your production server?

@viceice
Copy link
Author

viceice commented May 23, 2019

Normally nobody needs them for production

@paulmelnikow
Copy link
Collaborator

Ah yes, I see. Declaring it as a dependency allows us to trigger it to be installed automatically and match the version number. Making it a dev dependency of tmp-promise wouldn't work because dev dependencies wouldn't be installed. Making it a peer dependency would be no better, as it would yield warnings for non-TypeScript users. We could make it an optional dependency, though the consumer will still need to match the version on their own.

Until the types are shipped by tmp I'd still advocate making it a dependency, as it's the least error prone.

@benjamingr
Copy link
Owner

Until the types are shipped by tmp I'd still advocate making it a dependency, as it's the least error prone.

This means users' production environments are at-risk because of a devDependency which I am not a fan of (imagine a faulty postinstall script for example).

Honestly I think I'm fine with the status quo with the better alternative being tmp including its types which I think they're open to.

@paulmelnikow
Copy link
Collaborator

This means users' production environments are at-risk because of a devDependency which I am not a fan of (imagine a faulty postinstall script for example).

While I appreciate this concern, for what it's worth the packages that come from DefinitelyTyped are very reputable, published through an automation chain which is far removed from individual bad actors. I don't think it is possible even through a PR review process to inject scripts into those packages.

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

No branches or pull requests

3 participants