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

[7.0.0][type regression] Unpack is not assignable to NodeJS.WritableStream #409

Closed
AviVahl opened this issue Apr 11, 2024 · 11 comments
Closed

Comments

@AviVahl
Copy link

AviVahl commented Apr 11, 2024

This used to work with tar@6 and @types/tar:

import { createReadStream } from "node:fs";
import { mkdir } from "node:fs/promises";
import { pipeline } from "node:stream/promises";
import { extract } from "tar/extract";

export async function extractPackage(archivePath: string, targetPath: string) {
  await mkdir(targetPath, { recursive: true });
  await pipeline(
    createReadStream(archivePath),
    extract({
      cwd: targetPath,
      strip: 1,
    }),
  );
}

With v7 there's a type error about Unpack not being assignable to NodeJS.WritableStream.

Working around by casting it to as ReturnType<typeof extract> & NodeJS.WritableStream... as it seems to still work in runtime.

@codyebberson
Copy link

Same:

Argument of type 'Unpack' is not assignable to parameter of type 'WritableStream'.
  The types returned by 'write(...)' are incompatible between these types.
    Type 'boolean | Unzip | BrotliDecompress | undefined' is not assignable to type 'boolean'.
      Type 'undefined' is not assignable to type 'boolean'.ts(2345)

@maka-io
Copy link

maka-io commented Apr 17, 2024

Same:

error TS2345: Argument of type 'Unpack' is not assignable to parameter of type 'WritableStream'.
   The types returned by 'write(...)' are incompatible between these types.
     Type 'boolean | Unzip | BrotliDecompress | undefined' is not assignable to type 'boolean'.
       Type 'undefined' is not assignable to type 'boolean'.

@mattfysh
Copy link

I'm seeing the same error, but using the Parser class (which used to be named Parse but the change was not included in the changelog). Reproducible with the following:

readableStream.pipe(new Parser())

@isaacs
Copy link
Owner

isaacs commented Apr 25, 2024

I think the WritableStream type in @types/node must have changed recently, because I've seen this with everything that uses minipass, where it used to be no problem at all.

I'd like to find time to go through and update minipass to make TS know that it's compatible with the writable stream types in @types/node (because, in practice, it actually is), but to be honest, I don't see myself getting to that any time soon.

So, patch welcome (over there, not here), but in the meantime, just cast and go on with life.

I'm going to lock this thread to prevent a flood of "me too" comments. If someone wants to actually work on this, post an issue or PR over on https://github.com/isaacs/minipass to fix the types up to play nicer with @types/node.

Repository owner locked as off-topic and limited conversation to collaborators Apr 25, 2024
Repository owner unlocked this conversation May 4, 2024
@isaacs isaacs closed this as completed in 6b61030 May 4, 2024
@isaacs
Copy link
Owner

isaacs commented May 4, 2024

Ok, so this is one of those rare examples where my terrible inability estimate time meant I guessed it would take much longer to get to this than it actually did. (Much more often, I come across an issue ending with a comment from me saying I'd have some time to get to something "next week", posted 3 years ago or something lol)

This should be fixed in 7.1, please let me know if it is not.

It just required some very tedious type-checking for the annoyingly variadic optional arguments to write() and end(), but the maybe nice side-effect of it is that you can now also do parser.write(someHexString, 'hex') and it'll work as expected.f

@AviVahl
Copy link
Author

AviVahl commented May 4, 2024

Works. Thank you. The minipass dedupe is also appreciated.

@spraot
Copy link

spraot commented May 6, 2024

I'm using 7.1.0 and I'm seeing what looks like this issue:

  Type '(path?: string | ReadEntry | undefined) => this' is not assignable to type '{ (cb?: (() => void) | undefined): this; (chunk: Buffer, cb?: (() => void) | undefined): this; (chunk: Buffer, encoding?: Encoding | undefined, cb?: (() => void) | undefined): this; }'.
    Types of parameters 'path' and 'cb' are incompatible.
      Type '(() => void) | undefined' is not assignable to type 'string | ReadEntry | undefined'.
        Type '() => void' is not assignable to type 'string | ReadEntry | undefined'.

70     end(path?: string | ReadEntry): this;
       ~~~

../../../node_modules/tar/dist/commonjs/pack.d.ts:71:5 - error TS2416: Property 'write' in type 'Pack' is not assignable to the same property in base type 'Minipass<ContiguousData, Buffer, WarnEvent>'.
  Type '(path: string | ReadEntry) => boolean' is not assignable to type '{ (chunk: Buffer, cb?: (() => void) | undefined): boolean; (chunk: Buffer, encoding?: Encoding | undefined, cb?: (() => void) | undefined): boolean; }'.
    Types of parameters 'path' and 'chunk' are incompatible.
      Type 'Buffer' is not assignable to type 'string | ReadEntry'.
        Type 'Buffer' is missing the following properties from type 'ReadEntry': #private, header, startBlockSize, blockRemain, and 75 more.

71     write(path: string | ReadEntry): boolean;```

@spraot
Copy link

spraot commented May 6, 2024

However, I also see this issue on 6.2.1, so it could be something else.

@lguti97
Copy link

lguti97 commented May 6, 2024

^ also seeing a similar error

git:(main) ✗ npx tsc; node dist/index.js
node_modules/tar/dist/esm/pack.d.ts:70:5 - error TS2416: Property 'end' in type 'Pack' is not assignable to the same property in base type 'Minipass<ContiguousData, Buffer, WarnEvent>'.
  Type '(path?: string | ReadEntry) => this' is not assignable to type '{ (cb?: () => void): this; (chunk: Buffer, cb?: () => void): this; (chunk: Buffer, encoding?: Encoding, cb?: () => void): this; }'.
    Types of parameters 'path' and 'cb' are incompatible.
      Type '() => void' is not assignable to type 'string | ReadEntry'.

70     end(path?: string | ReadEntry): this;
       ~~~

node_modules/tar/dist/esm/pack.d.ts:71:5 - error TS2416: Property 'write' in type 'Pack' is not assignable to the same property in base type 'Minipass<ContiguousData, Buffer, WarnEvent>'.
  Type '(path: string | ReadEntry) => boolean' is not assignable to type '{ (chunk: Buffer, cb?: () => void): boolean; (chunk: Buffer, encoding?: Encoding, cb?: () => void): boolean; }'.
    Types of parameters 'path' and 'chunk' are incompatible.
      Type 'Buffer' is not assignable to type 'string | ReadEntry'.
        Type 'Buffer' is missing the following properties from type 'ReadEntry': #private, header, startBlockSize, blockRemain, and 75 more.

71     write(path: string | ReadEntry): boolean;
       ~~~~~


Found 2 errors in the same file, starting at: node_modules/tar/dist/esm/pack.d.ts:70

@viceice
Copy link

viceice commented May 13, 2024

Also still seeing this error:

node_modules/.pnpm/tar@7.1.0/node_modules/tar/dist/commonjs/pack.d.ts:70:5 - error TS2416: Property 'end' in type 'Pack' is not assignable to the same property in base type 'Minipass<ContiguousData, Buffer, WarnEvent>'.
  Type '(path?: string | ReadEntry | undefined) => this' is not assignable to type '{ (cb?: (() => void) | undefined): this; (chunk: Buffer, cb?: (() => void) | undefined): this; (chunk: Buffer, encoding?: Encoding | undefined, cb?: (() => void) | undefined): this; }'.
    Types of parameters 'path' and 'cb' are incompatible.
      Type '(() => void) | undefined' is not assignable to type 'string | ReadEntry | undefined'.
        Type '() => void' is not assignable to type 'string | ReadEntry | undefined'.

70     end(path?: string | ReadEntry): this;
       ~~~

node_modules/.pnpm/tar@7.1.0/node_modules/tar/dist/commonjs/pack.d.ts:71:5 - error TS2416: Property 'write' in type 'Pack' is not assignable to the same property in base type 'Minipass<ContiguousData, Buffer, WarnEvent>'.
  Type '(path: string | ReadEntry) => boolean' is not assignable to type '{ (chunk: Buffer, cb?: (() => void) | undefined): boolean; (chunk: Buffer, encoding?: Encoding | undefined, cb?: (() => void) | undefined): boolean; }'.
    Types of parameters 'path' and 'chunk' are incompatible.
      Type 'Buffer' is not assignable to type 'string | ReadEntry'.
        Type 'Buffer' is missing the following properties from type 'ReadEntry': #private, header, startBlockSize, blockRemain, and 75 more.

71     write(path: string | ReadEntry): boolean;
       ~~~~~


Found 2 errors in the same file, starting at: node_modules/.pnpm/tar@7.1.0/node_modules/tar/dist/commonjs/pack.d.ts:70

we're using @types/node@18.19.31

@isaacs
Copy link
Owner

isaacs commented May 13, 2024

@viceice can you please open a new issue with reproduction steps? The more minimal the better. I suspect it has something to do with the typescript and/or @types/node versions in play.

Repository owner locked and limited conversation to collaborators May 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants