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

Typescript: All events eventually extend from 'IValueMap' making it difficult correctly type any event #388

Open
Meess opened this issue Aug 14, 2023 · 1 comment

Comments

@Meess
Copy link

Meess commented Aug 14, 2023

Short version:

  • It would be nice if the events don't extend from IValueMap, that would make it possible for Typescript to properly infer the types.
  • And to export used types, i.e. atleast IMidiFile, Tracks and all events.

Long version:

To take one event as example:
IMidiSetTempoEvent

IMidiSetTempoEvent eventually extends TValue which can be a lot (see extending type chain below). This make it extremely hard to determine which event we're looking when have file of type IMidiFile.

For example I have a function that takea a IMidiFile as input first the type IMidiFile needs to be derived as it's not exported , i.e.:

# So now IMidiFile can be used in the codebase that imports midi-json-parser
export type IMidiFile = Awaited<ReturnType<typeof parseArrayBuffer>>;

const someFunction = (file: IMidiFile) => { ... }

IMidiFile contains the tracks, typed as Tracks[][], the Tracks Type itself is not exported hence should also be derived, but is a bit more complex:

# Make a custom type that can get the type of a list type (i.e. Unpacks<Tracks[]> will become Tracks)
type Unpacked<T> = T extends (infer U)[] ? U : T;

# Double unpack Tracks[][] to get the Tracks type:
export type Tracks = Unpacked<Unpacked<IMidiFile['tracks']>>;

Now we can Tracks an input type for a function in our own codebase. If we create a loop over the events in a track, it's difficult to get the typing right for that event. It might be possible to do the following:

if('setTempo' in Event){ 
    microsecondsPerQuarter =  Event.setTempo.microsecondsPerQuarter;
} }

But that will give a type error, because the type where 'setTempo' exists on is IMidiSetTempoEvent, which extends IMidiMetaEvent, which extends IValueMap which have string keys and TValue as value. See the following chain:

// (midi-json-parser-worker): midi-set-tempo-event.ts
export interface IMidiSetTempoEvent extends IMidiMetaEvent {
    setTempo: {
        microsecondsPerQuarter: number;
    };
}

// (midi-json-parser-worker): midi-meta-event.ts
export interface IMidiMetaEvent extends IValueMap {
    delta: number;
}

// (worker-factory): value-map.ts
export interface IValueMap {
    [key: string]: TValue;
}

// (worker-factory): value.ts
export type TValue = boolean | null | number | string | IValueArray | RegExp | TTypedArray | TValueMap | Transferable;

which basically means IMidiSetTempoEvent can have any key, and can have any value of TValue. So with our check 'setTempo' in Event TypeScript can't simplify the type because 'setTempo' is both a valid key for IMidiSetTempoEvent and IValueMap (because it fits the key type string).

So accessing Event.setTempo.microsecondsPerQuarter will give an error because setTempo can also be a TValue, and TValue doesn't specify an object so it will error saying it can't access microsecondsPerQuarter, i.e. the error:

Property 'microsecondsPerQuarter' does not exist on type 'string | number | boolean | RegExp | ArrayBuffer | IValueArray | Float32Array | Float64Array | Int8Array | ... 14 more ... | { ...; }'.
  Property 'microsecondsPerQuarter' does not exist on type 'string'

This makes it very difficult to properly check for an event and have the proper typing of the event to handle it, because basically every event becomes a big IValueMap which can be almost anything (except for an object).

@alexshenker
Copy link

alexshenker commented Mar 29, 2024

I ran into this, and I am left with the assumption that there's reasoning behind that typing.
Here is a workaround that will make it easier for you.

For each kind of even you'd like to check for, create a function that will

  1. check the event type based on the key
  2. if the event type is correct, it will return the event Typed as the specific event you're checking for, or null

typing the event directly as one of the specific MidiEvents types avoids the IValue issue for some reason.

EX:

import {
  TMidiEvent,
  IMidiNoteOnEvent,
  IMidiNoteOffEvent,
} from "midi-json-parser-worker";

type EventKeysToCheck = "noteOn" | "noteOff";

function typedMidiEvent(
  eventKeyToCheck: "noteOn",
  event: TMidiEvent,
): IMidiNoteOnEvent | null;

function typedMidiEvent(
  eventKeyToCheck: "noteOff",
  event: TMidiEvent,
): IMidiNoteOffEvent | null;

function typedMidiEvent(eventKeyToCheck: EventKeysToCheck, event: TMidiEvent) {
  if (eventKeyToCheck in event) {
    return event;
  }

  return null;
}

Then ts should work normally:
Screenshot 2024-03-29 at 5 47 57 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants