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

make @types/readable-stream a standard dependency #133

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

kyle-johnson
Copy link
Contributor

The builtin types reference @types/readable-stream but don't include it as a dependency (it's currently in devDependencies) meaning consumers of this package must explicitly include @types/readable-stream in their own dependencies for types to resolve properly. Without this fix, tsc gives errors like the following:

  Overload 1 of 7, '(source: Readable, destination: WritableStream | PipelineDestinationIterableFunction<any> | PipelineDestinationPromiseFunction<any, any>, options?: PipelineOptions | undefined): Promise<...> | Promise<...>', gave the following error.
    Argument of type 'BufferListStream' is not assignable to parameter of type 'WritableStream | PipelineDestinationIterableFunction<any> | PipelineDestinationPromiseFunction<any, any>'.
  Overload 2 of 7, '(streams: readonly (WritableStream | ReadableStream | ReadWriteStream)[], options?: PipelineOptions | undefined): Promise<...>', gave the following error.
    Argument of type 'Readable' is not assignable to parameter of type 'readonly (WritableStream | ReadableStream | ReadWriteStream)[]'.
      Type 'Readable' is missing the following properties from type 'readonly (WritableStream | ReadableStream | ReadWriteStream)[]': length, concat, join, slice, and 19 more.
  Overload 3 of 7, '(stream1: ReadableStream, stream2: WritableStream | ReadWriteStream, ...streams: (WritableStream | ReadWriteStream | PipelineOptions)[]): Promise<...>', gave the following error.
    Argument of type 'BufferListStream' is not assignable to parameter of type 'WritableStream | ReadWriteStream'.

Moving type dependencies into dependencies matches the guidance in Typescript docs https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html#dependencies

Make sure all the declaration packages you depend on are marked appropriately in the "dependencies" section in your package.json. For example, imagine we authored a package that used Browserify and TypeScript. [...] browserify does not bundle its declaration files with its npm packages, so we needed to depend on @types/browserify for its declarations.

Copy link
Owner

@rvagg rvagg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL, thanks for the TS handbook link!

@rvagg rvagg merged commit 161c378 into rvagg:master Jan 10, 2024
18 checks passed
Copy link

github-actions bot commented Feb 8, 2024

🎉 This PR is included in version 6.0.11 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants