GMOD / bam-js

Parse BAM and BAM index files in javascript for node and the browser
MIT License
19 stars 9 forks source link

Typing issues #97

Open tuner opened 1 year ago

tuner commented 1 year ago

Hi,

I'm trying to validate types (I'm using TS through JSDoc) in my codebase using tsc, but I stumbled upon a couple of issues in @GMOD/bam@1.1.18.

The first one:

../../node_modules/@gmod/bam/dist/util.d.ts:4:44 - error TS2304: Cannot find name 'Long'.

4 export declare function longToNumber(long: Long): number;
                                             ~~~~

There's no "long" package imported in util.d.ts (https://cdn.jsdelivr.net/npm/@gmod/bam@1.1.18/dist/util.d.ts), although it's in the original source file: https://github.com/GMOD/bam-js/blob/492c145a3a714d898b3a38f82030876900ad7ede/src/util.ts#L1

🤷

If I checkout v1.1.18 of this repo and compile the project, the long import is missing from util.d.ts. If I checkout HEAD and compile, then it's there. Maybe there's some problem in the old typescript@4.9.4 (?)

And then there's the other issue:

../../node_modules/@gmod/bam/dist/htsget.d.ts:14:5 - error TS2416: Property '_readChunk' in type 'HtsgetFile' is not assignable to the same property in base type 'BamFile'.
  Type '(params: { chunk: { buffer: Buffer; chunk: Chunk; }; opts: BaseOpts; }) => Promise<{ data: Buffer; cpositions: null; dpositions: null; chunk: Chunk; }>' is not assignable to type '({ chunk, opts }: { chunk: Chunk; opts: BaseOpts; }) => Promise<{ data: any; cpositions: any; dpositions: any; chunk: Chunk; }>'.
    Types of parameters 'params' and '__0' are incompatible.
      Type '{ chunk: Chunk; opts: BaseOpts; }' is not assignable to type '{ chunk: { buffer: Buffer; chunk: Chunk; }; opts: BaseOpts; }'.
        Types of property 'chunk' are incompatible.
          Type 'Chunk' is missing the following properties from type '{ buffer: Buffer; chunk: Chunk; }': buffer, chunk

14     _readChunk(params: {
       ~~~~~~~~~~

Indeed, they are different: https://github.com/GMOD/bam-js/blob/492c145a3a714d898b3a38f82030876900ad7ede/src/bamFile.ts#L401 https://github.com/GMOD/bam-js/blob/492c145a3a714d898b3a38f82030876900ad7ede/src/htsget.ts#L110

However, I don't have enough knowledge of TypeScript to say whether these should be compatible or not. 🤔

And btw, I'm using typescript@5.0.4.

cmdcolin commented 1 year ago

I recently did some work on typing in https://github.com/GMOD/bam-js/pull/96 as there was indeed a need for some improvements. It may culminate with a major version bump, pretty minor changes to the api but changes nonetheless

I'm not sure about the long thing, sometimes that can imply that I need to include @types/long in the actual dependencies of @gmod/bam instead of just the devDependencies

I would have to assess if there are any remaining type improvements that can be done

cmdcolin commented 1 year ago

Just curious are you using skipLibCheck:true or false?

tuner commented 1 year ago

It's on the default setting (false?). It seems that I can get rid of the error message by setting it to true. Is it a good long-term solution?

cmdcolin commented 1 year ago

We can likely aim to fix usage of @gmod/bam with skipLibCheck:false (e.g. the default) but I have used skipLibCheck:true in a lot of my projects that consume libraries like @gmod/bam (not ideal, but a quick workaround for stuff like this)

tuner commented 1 year ago

Thanks, Colin! It seems that skipLibCheck skips those parts of library type definitions I'm not using anywhere, so, I think I can safely enable it for now.