node-tar icon indicating copy to clipboard operation
node-tar copied to clipboard

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

Open AviVahl opened this issue 2 months ago • 4 comments

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.

AviVahl avatar Apr 11 '24 12:04 AviVahl

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)

codyebberson avatar Apr 15 '24 23:04 codyebberson

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'.

maka-io avatar Apr 17 '24 15:04 maka-io

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())

mattfysh avatar Apr 25 '24 04:04 mattfysh

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.

isaacs avatar Apr 25 '24 15:04 isaacs

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

isaacs avatar May 04 '24 02:05 isaacs

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

AviVahl avatar May 04 '24 02:05 AviVahl

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 avatar May 06 '24 14:05 spraot

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

spraot avatar May 06 '24 14:05 spraot

^ 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

lguti97 avatar May 06 '24 19:05 lguti97

Also still seeing this error:

node_modules/.pnpm/[email protected]/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/[email protected]/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/[email protected]/node_modules/tar/dist/commonjs/pack.d.ts:70

we're using @types/[email protected]

viceice avatar May 13 '24 13:05 viceice

@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.

isaacs avatar May 13 '24 14:05 isaacs