TypeScript compile error because of incompatible override of Atomic.unpack by its subclasses

[soimon]

I am a TypeScript user with strict mode turned on, and in that configuration this library doesn't compile without manual modifications. I would like to address that issue for fellow strict TypeScript users, to allow them access to this otherwise great library.

❗ Behavior

When trying to install and thus compile the library with TypeScript, the following error occurs:

node_modules/osc-js/lib/osc.d.ts:200:5 - error TS2416: Property 'unpack' in type 'AtomicTimetag' is not assignable to the same property in base type 'Atomic'.
  Type '(dataView: DataView, initialOffset?: number | undefined) => number' is not assignable to type '(dataView: DataView, method: string, byteLength: number, initialOffset?: number | undefined) => number'.
    Types of parameters 'initialOffset' and 'method' are incompatible.
      Type 'string' is not assignable to type 'number'.

❔ Cause

AtomicTimetag is overriding its parent method unpack in an incompatible way. Parent method Atomic.unpack is defined as

unpack(dataView: DataView, method: string, byteLength: number, initialOffset?: number): number;

While AtomicTimetag.unpack is defined as

unpack(dataView: DataView, initialOffset?: number): number;

By omitting the method and byteLength property, it renders itself incompatible with its base class, preventing TypeScript from compiling.

✔️ How to fix

By changing the child signature into

unpack(dataView: DataView, initialOffset?: number|string): number;

it overlaps with the parent signature enough to compile. However, this is kinda hacky and the child classes should not override their base methods incompatibly.

[adzialocha]

Thank you @soimon for bringing this up and sorry for the delayed response (have been busy with a larger long-term open source project I'm contributing to).

I think I have a fix ready at #78. Maybe you can double-check that this really did the job? Thank you!

I've decided to not change the child signature but rename the base unpack method, as I think TypeScript rightly complains that they're just of different nature and they shouldn't be seen as the same.

[soimon]

Thank you @soimon for bringing this up and sorry for the delayed response (have been busy with a larger long-term open source project I'm contributing to).

I think I have a fix ready at #78. Maybe you can double-check that this really did the job? Thank you!

I've decided to not change the child signature but rename the base unpack method, as I think TypeScript rightly complains that they're just of different nature and they shouldn't be seen as the same.

Thanks for the update! I can confirm both this issue and #75 are fixed on my end. Speaking about timing: the performance I needed this fix for will premiere at the end of the week, so your timing is flawless and highly appreciated.