isaacs / node-tar

tar for node
ISC License
837 stars 183 forks source link

Incompatible TS types `Pack` and `Minipass<...>` #421

Closed Goodwine closed 2 months ago

Goodwine commented 2 months ago

Repro:

https://gist.github.com/Goodwine/4cc0f2c078c6fb163006633c0ef48fad

Copy the files from the gist and run:

npm install
npx tsc

Error:

node_modules/tar/dist/commonjs/pack.d.ts:71: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'.

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

node_modules/tar/dist/commonjs/pack.d.ts:72: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.

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

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

This has been called out on other bugs that were marked as closed, however the issue is still present as seen on the repro steps:

Goodwine commented 2 months ago

Upon further investigation, the issue is that the pack.ts file is disabling the TS compiler with @ts-ignore, which downstream dependencies do not ignore because the generated .d.ts types do not include this comment.

So the solution should be removing // @ts-ignore or generating types that include the compiler-check-disabling comment

Goodwine commented 2 months ago

According to the types, I believe the compiler error is actually correct and shouldn't be disabled. Whether that means that Minipass or Tar type declarations are incorrect, I don't know, but one of those is incorrect.

isaacs commented 2 months ago

Thanks for the repro case, that made it much clearer!

Should work fine on 7.4.2.

stephematician commented 2 months ago

Thanks for the repro case, that made it much clearer!

Should work fine on 7.4.2.

Resolved what I had observed on my system - thanks!