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

Improve type checking #25

Closed
vasilii-kovalev opened this issue Jul 7, 2021 · 10 comments · Fixed by #26
Closed

Improve type checking #25

vasilii-kovalev opened this issue Jul 7, 2021 · 10 comments · Fixed by #26
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@vasilii-kovalev
Copy link
Owner

vasilii-kovalev commented Jul 7, 2021

generatePath function in React Router library supports variable names check, comparing ones from the route string and ones from the variables object.
Types: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react-router/index.d.ts#L146-L166

It would be great to implement similar type checking in this library.

I tried to do that myself and found, that the approach above works good in case of fixed prefix and suffix values. Otherwise, it is hard to handle edge cases like these:

  • prefix and suffix are empty strings (passed explicitly or passed interpolation options object is empty)

  • Either preffix or suffix is passed (the other becomes an empty string by default, see docs or implementation)

  • prefix and/or suffix are set in configureHydrateText and then overridden in the result function:

    const routeWithCustomInterpolationOptions = "/users/(userId)";
    
    const hydrateRoute = configureHydrateText({ prefix: ":" });
    
    // '/users/3'
    console.log(
      hydrateRoute(
        routeWithCustomInterpolationOptions,
        { userId: 3 },
        {
          prefix: "(",
          suffix: ")",
        },
      ),
    );

Definition of done:

  • Functionality haven't suffered
  • Type checking has improved
  • Linters checks and tests are passing
  • Code coverage is 100%
@vasilii-kovalev vasilii-kovalev added enhancement New feature or request help wanted Extra attention is needed labels Jul 7, 2021
@ghost
Copy link

ghost commented Jul 7, 2021

It would be great to have some reproducible example on TypeScript Playground, so I can experiment with it and not getting myself into the context (that takes much more time). Promise nothing, but I can try to understand what you want to achieve and find the solution.

@vasilii-kovalev
Copy link
Owner Author

Created a playground with a bunch of cases of type checking that should fail with improved types.

@ghaiklor
Copy link

ghaiklor commented Jul 7, 2021

Thanks for a detailed playground, it is really helpful!

I had a quick look and from what I understood, you need to create a "parser" on type-level that operates on string type literals. For that, you can inspect into template literal types.

Unfortunately, I can't say that it is ezpz work. It is a huge epic, but it is doable. Here, for example, a simple parser to extract the name inside the curly block:

type ParseCurlyBlock<T> = T extends `{${infer N}}` ? N : never
type R = ParseCurlyBlock<'{username}'> // R = "username"

Afterwards, you can combine these parsers together with recursive types to get a list of all variables inside your template and collect it as a union of possible variables that reflects the state of "what variables can I pass in".

One drawback that I already see is that you can't configure them in runtime as you do with prefix or suffix, because you need to know what is the prefix and suffix at compile time. You can overcome that with constant values and it seems like it will work.

I will have some spare time at these weekends and could take a closer look and implement some basic implementation of typed templates. I hope this implementation will give you some insights on how to go further with it. Until then, please, make yourself comfortable with template literal types because you really need this in order to achieve what you want (unless I misunderstood something).

@vasilii-kovalev
Copy link
Owner Author

vasilii-kovalev commented Jul 7, 2021

Thank you! I'm aware of the template literal types, and they are used in generatePath type definition (see the very beginning of this issue's message). The problem is in the "dynamic" nature of the markers (prefix/suffix).

@ghaiklor
Copy link

ghaiklor commented Jul 7, 2021

Unless you don't parametrize the type to hold prefix and suffix type literals 😉

type Parser<P extends string, S extends string, I> = I extends `${P}${infer N}${S}` ? N : never

type R0 = Parser<"{", "}", "{username}">
type R1 = Parser<"(", ")", "(username)">

What is blocking you from specifying your prefix and suffix as type parameters and Typescript will infer automatically from usage? 🙂

@ghaiklor
Copy link

ghaiklor commented Jul 7, 2021

JFYI, I've already solved the task, but I'm wondering if you got the idea and did it yourself 😉
I'll post a solution tomorrow on my Telegram channel with step-by-step explanation. It is a good one task indeed.

@vasilii-kovalev
Copy link
Owner Author

vasilii-kovalev commented Jul 8, 2021

@ghaiklor, yes, I did basically the same, but there are problems with the edge cases I mentioned in this issue' message.
I'll try to create my implementation in an hour and share it here.

@vasilii-kovalev
Copy link
Owner Author

vasilii-kovalev commented Jul 8, 2021

That's my best try at the moment. It works for very simple cases, but fails in a little more complex ones (especially, with prefix/suffix as empty strings).

type ValueType = string | number | boolean;

type Variables = Record<string, ValueType>;

interface InterpolationOptions<Prefix extends string = string, Suffix extends string = string> {
  prefix?: Prefix;
  suffix?: Suffix;
}

type GetVariables<
  Text extends string,
  Prefix extends string,
  Suffix extends string
> = string extends Text
  ? { [variable in string]?: ValueType }
  : Text extends `${infer _Start}${Prefix}${infer Variable}${Suffix}${infer Rest}`
    ? { [variable in Variable]: ValueType } & GetVariables<Rest, Prefix, Suffix>
    : {};

interface HydrateText {
  <
    Text extends string = string,
    Prefix extends string = "{",
    Suffix extends string = "}",
  >(
    text: Text,
    variables?: GetVariables<Text, Prefix, Suffix>,
    interpolationOptions?: InterpolationOptions<Prefix, Suffix>,
  ): string;
}

interface ConfigureHydrateText {
  <
    _Prefix extends string = "{",
    _Suffix extends string = "}"
  >(interpolationOptions?: InterpolationOptions<_Prefix, _Suffix>): <
    Text extends string = string,
    Suffix extends string = _Suffix,
    Prefix extends string = _Prefix,
  >(
    text: Text,
    variables?: GetVariables<Text, Prefix, Suffix>,
    interpolationOptions?: InterpolationOptions<Prefix, Suffix>,
  ) => string;
}

const DEFAULT_INTERPOLATION_OPTIONS: InterpolationOptions<"{", "}"> = {
  prefix: "{",
  suffix: "}",
};

const EMPTY_INTERPOLATION_OPTIONS: InterpolationOptions<"", ""> = {
  prefix: "",
  suffix: "",
};

const hydrateText: HydrateText = (
  text,
  variables,
  interpolationOptions,
) => {
  const { prefix, suffix } = interpolationOptions
    ? {
        ...EMPTY_INTERPOLATION_OPTIONS,
        ...interpolationOptions,
      }
    : DEFAULT_INTERPOLATION_OPTIONS;

  const resultText = Object.entries(variables ?? {}).reduce(
    (resultText, [name, value]) => {
      const regExpSource = escapeRegExp(`${prefix}${name}${suffix}`);
      const regExp = new RegExp(regExpSource, "g");

      return resultText.replace(regExp, String(value));
    },
    text as string,
  );

  return resultText;
};

const configureHydrateText: ConfigureHydrateText =
  interpolationOptionsFromConfigurer =>
  (text, variables, interpolationOptionsFromInnerFunction) => {
    const interpolationOptions =
      interpolationOptionsFromInnerFunction ??
      interpolationOptionsFromConfigurer;

    /*
      `interpolationOptions` throws an error:
      Argument of type 'InterpolationOptions<_Prefix, _Suffix> | InterpolationOptions<Prefix, Suffix> | undefined'
      is not assignable to parameter of type 'InterpolationOptions<Prefix, Suffix> | undefined'.
    */
    return hydrateText(text, variables, interpolationOptions);
  };

Playground.

@ghaiklor
Copy link

ghaiklor commented Jul 8, 2021

prefix and suffix are empty strings (passed explicitly or passed interpolation options object is empty)

These cases need to be handled explicitly via P extends "" ? <P is empty> : <P is not empty>.

Either preffix or suffix is passed (the other becomes an empty string by default, see docs or implementation)

Why an empty string? But, anyway, if so, see ☝🏻 I'd make some char by default, not the empty string.

prefix and/or suffix are set in configureHydrateText and then overridden in the result function:

You can try to pack the P and S types in resulting function from configureHydrateText and override them later via type parameters.

@ghaiklor
Copy link

ghaiklor commented Jul 8, 2021

Playground

@vasilii-kovalev vasilii-kovalev self-assigned this Jul 13, 2021
Repository owner locked as resolved and limited conversation to collaborators Jul 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants