fwextensions / remotion-time

MIT License
15 stars 1 forks source link

Strict TS types #3

Open apolubek opened 11 months ago

apolubek commented 11 months ago

In Remotion projects with TypeScript, the type annotations of provided hooks are not strict meaning that e.g.

import {
  useTimeConfig,
  useInterpolate,
  useTime,
} from "remotion-time";
function TestComponent() {
  const config = useTimeConfig("saggsadgsadg"); // is possible
  const opacity = useInterpolate(["start", "end"], [0, 1]); //  typeof opacity is number | {} | Record<string, number>
  const blur = useInterpolate(["wrong", "values", "are", "possible"], [0, 1]);
  const t = useTime();
  t`sdgasgdasgd`; // is possible
}

It would be awesome for TS users to have better type annotations dependent on the input values. I created wrapper on current remotion-time functions so developer's experience is much better for TS projects.

import {
  useTimeConfig as _useTimeConfig,
  useInterpolate as _useInterpolate,
  useTime as _useTime,
  InterpolateOptions,
} from "remotion-time";

type TimeUnit =
  | "s"
  | "sec"
  | "second"
  | "seconds"
  | "m"
  | "min"
  | "minute"
  | "minutes"
  | "h"
  | "hr"
  | "hour"
  | "hours"
  | "%"
  | "pct";

type RelativeUnit =
  | "start"
  | "beginning"
  | "middle"
  | "half"
  | "end"
  | "length"
  | "duration";

type TimeString =
  | `${number}${TimeUnit}`
  | RelativeUnit
  | `${RelativeUnit} + ${number}${TimeUnit}`
  | `${RelativeUnit} - ${number}${TimeUnit}`;

type TimeStringOrNumber = TimeString | number;

type ConfigString = `${number}${TimeUnit} @ ${number}fps`;

export const useTimeConfig = (configString: ConfigString) =>
  _useTimeConfig(configString);

export function useInterpolate<
  Input extends (readonly [] | readonly TimeStringOrNumber[]) &
    (number extends Input["length"] ? readonly [] : unknown)
>(
  input: Input,
  output: { [I in keyof Input]: number },
  options?: InterpolateOptions
): number;

export function useInterpolate<
  Input extends (readonly [] | readonly TimeStringOrNumber[]) &
    (number extends Input["length"] ? readonly [] : unknown),
  Keys extends string
>(
  input: Input,
  output: { [K in Keys]: { [I in keyof Input]: number } },
  options?: InterpolateOptions
): { [K in Keys]: number };

export function useInterpolate<
  Input extends (readonly [] | readonly TimeStringOrNumber[]) &
    (number extends Input["length"] ? readonly [] : unknown)
>(input: Input, output: any, options?: InterpolateOptions) {
  return _useInterpolate(input, output, options);
}

export const useTime = () => {
  const t = _useTime();
  return (timeString: TimeString) => t`${timeString}`;
};

And the result is like follows

function TestComponent() {
  // useTimeConfig
  const timeConfig1 = useTimeConfig("20 seconds @ 30fps");
  const timeConfig2 = useTimeConfig("2seconds at 24 fps"); // error! Type '"2seconds at 24 fps"' is not assignable to type ...

  // useInterpolate
  const i1 = useInterpolate(["start", "end - 1s"], [3, 4]); // typeof i1 is number
  const i2 = useInterpolate([1, 2], {
    opacity: [1, 2],
    blur: [0, 40],
  }); // typeof i2 is { opacity: number; blur: number; }

  const i3 = useInterpolate([1, "end - end"], [0, 1]); // error! Type "end - end" is not assignable to type TimeStringOrNumber
  const i4 = useInterpolate([1, 2], [3, 5, 3]); // error! Argument of type [number, number, number] is not assignable to parameter of type  [number, number]
  const i5 = useInterpolate(["start", "middle - 1s", "end"], {
    opacity: [1, 2, 2],
    blur: [0],
  }); // error! Type [number] is not assignable to type [number, number, number]

  // useTime
  const t = useTime();

  const t1 = t("20.2 min");
  const t2 = t("sagasggas"); // error! Type '"sagasggas"' is not assignable to type TimeString
  const t3 = t("200"); // error! Type '"200"' is not assignable to type TimeString
  const t4 = t(200); // error! Type '200' is not assignable to type TimeString
}
Screenshot 2023-09-12 at 16 18 08
fwextensions commented 11 months ago

Sorry for the slow response. I missed this notification among a bunch of others.

This is great! I'm fairly new at TS, so didn't know about things like template literal types.

Would you want to open a PR to make these changes? If not, I think I can take what you've done and apply it.

apolubek commented 11 months ago

Sorry for the slow response. I missed this notification among a bunch of others.

No worries, we all have too many of those 😅

Would you want to open a PR to make these changes? If not, I think I can take what you've done and apply it.

Feel free to apply those changes and tweak them if necessary. I'm sure it'll take you less time as you're more familiar with the source code. If you need any help, let me know and I'll gladly do that. 🙂

fwextensions commented 11 months ago

I'm curious about this in the useInterpolate definition:

  Input extends (readonly [] | readonly TimeStringOrNumber[]) &
    (number extends Input["length"] ? readonly [] : unknown),

Why allow a generic array? Is that to allow an empty array? I think this should have at least one time string or number.

Also not sure why this would need to be intersected with readonly [], if it's already typed as readonly TimeStringOrNumber[].

apolubek commented 11 months ago

It was based on the answer posted here: https://stackoverflow.com/a/62206961. It's just to give TS compiler a hint to treat those arrays as tuple types, e.g. [TimeStringOrNumber, TimeStringOrNumber] in case when array of length 2 is passed and not as TimeStringOrNumber[].

Why allow a generic array? Is that to allow an empty array? I think this should have at least one time string or number.

Great idea! I've tweaked the implementation a bit so it's not allowed to pass empty array. The implementation is much simpler now, as after forcing to use tuple instead of array using NonEmptyArray type, it's no need to use & (number extends Input["length"] ? readonly [] : unknown) suffix to the input type. Here's simplified version:

import {
  useTimeConfig as _useTimeConfig,
  useInterpolate as _useInterpolate,
  useTime as _useTime,
  InterpolateOptions,
} from "remotion-time";

type TimeUnit =
  | "s"
  | "sec"
  | "second"
  | "seconds"
  | "m"
  | "min"
  | "minute"
  | "minutes"
  | "h"
  | "hr"
  | "hour"
  | "hours"
  | "%"
  | "pct";

type RelativeUnit =
  | "start"
  | "beginning"
  | "middle"
  | "half"
  | "end"
  | "length"
  | "duration";

type TimeString =
  | `${number}${TimeUnit}`
  | RelativeUnit
  | `${RelativeUnit} + ${number}${TimeUnit}`
  | `${RelativeUnit} - ${number}${TimeUnit}`;

type TimeStringOrNumber = TimeString | number;

type ConfigString = `${number}${TimeUnit} @ ${number}fps`;

type NumberArrayOfLengthAs<Arr extends readonly unknown[]> = {
  [K in keyof Arr]: number;
};

type NonEmptyArray<T> = readonly [T, ...T[]];

export const useTimeConfig = (configString: ConfigString) =>
  _useTimeConfig(configString);

export function useInterpolate<Input extends NonEmptyArray<TimeStringOrNumber>>(
  input: Input,
  output: NumberArrayOfLengthAs<Input>,
  options?: InterpolateOptions
): number;

export function useInterpolate<
  Input extends NonEmptyArray<TimeStringOrNumber>,
  Keys extends string
>(
  input: Input,
  output: { [K in Keys]: NumberArrayOfLengthAs<Input> },
  options?: InterpolateOptions
): { [K in Keys]: number };

export function useInterpolate<Input extends NonEmptyArray<TimeStringOrNumber>>(
  input: Input,
  output: any,
  options?: InterpolateOptions
) {
  return _useInterpolate(input, output, options);
}

export const useTime = () => {
  const t = _useTime();
  return (timeString: TimeString) => t`${timeString}`;
};
fwextensions commented 11 months ago

Thanks, that's a lot easier to understand! Nice to learn more complex real-world TS...