Skip to content

Type instantiation is excessively deep not reported with a complex type involving a conditional type and another aliased conditional type #50017

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

Closed
Andarist opened this issue Jul 23, 2022 · 7 comments
Labels
Not a Defect This behavior is one of several equally-correct options

Comments

@Andarist
Copy link
Contributor

Bug Report

πŸ”Ž Search Terms

NonNullable, conditional type, excessively deep

πŸ•— Version & Regression Information

  • This changed between versions 4.2 and 4.3 and I've bisected it to this PR

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

type StringKeys<O> = Extract<keyof O, string>;
type Values<O> = O[keyof O];

type If<Condition, Then, Else> = Condition extends true ? Then : Else;
type IsObject<T> = T extends Record<string, any> ? true : false;
type IsArray<T> = T extends Array<any> ? true : false;

interface XataRecord {
  id: string;
}

type MAX_RECURSION = 5;

type NestedColumns<O, RecursivePath extends any[]> =
  // this check is important too
  RecursivePath["length"] extends MAX_RECURSION
    ? never
    : Values<
        {
          [K in DataProps<O>]: If<
            IsArray<O[K]>,
            K,
            // this NonNullable is important
            NonNullable<O[K]> extends XataRecord
              ? // NestedColumns (recursive) here seems to be important
                NestedColumns<
                  // this NonNullable is important
                  NonNullable<O[K]>,
                  // both of those are important
                  [...RecursivePath, O[K]]
                > extends string
                ? `${NestedColumns<
                    NonNullable<O[K]>,
                    // both of those are important
                    [...RecursivePath, O[K]]
                  >}`
                : never
              : never
          >;
        }
      >;

type DataProps<O> = Exclude<keyof O & string, keyof XataRecord>;
// it errors with this variant
// type DataProps<O> = keyof O & string;

πŸ™ Actual behavior

It doesn't error on the template literal type.

πŸ™‚ Expected behavior

It should error on the template literal type.

I believe that it should error because:

  1. it was an error in 4.2
  2. a bigger repro of this started to error in nightly versions since the NonNullable change (from a conditional type to T & {}). The slimmed down version that I've created doesn't error in the nightly build though. Original repro by @SferaDev can be found here: 4.7 with no error, 4.8 nightly with the error
  3. If we inline the If<IsArray<O[K]>, ..., ...> alias then it starts to error in all of the versions of the slimmed down repro: TS playground. To the best of my knowledge the aliased version and inlined version should not differ in their observable behaviors.

I understand that the repro is not small - I've tried to reduce it further but I couldn't. The problem at hand seems to be quite complex and involves a very specific ingredients from what I can tell.

@GuodongFan
Copy link

bug

@RyanCavanaugh
Copy link
Member

It should error on the template literal type

Wait, what?

Generally we do lots of effort to avoid issuing this error; it's always bad. Why is issuing the error desirable here?

@SferaDev
Copy link

Generally we do lots of effort to avoid issuing this error; it's always bad. Why is issuing the error desirable here?

Yeah, actually my original repro was to try to remove this error for good. And that's when @Andarist found that there was a behavioral difference between both options.

@Andarist
Copy link
Contributor Author

Generally we do lots of effort to avoid issuing this error; it's always bad. Why is issuing the error desirable here?

I don't mind this not being an error. The only thing that I care about it here is consistency. I've just figured out that it might be preferable (or at least easier) to make this an error rather than to make the error go away for all of those cases.

@RyanCavanaugh RyanCavanaugh added the Not a Defect This behavior is one of several equally-correct options label Jul 29, 2022
@RyanCavanaugh
Copy link
Member

I don't think there's anything to fix here. For any potential analytical situation where there's a problem (infinite instantiation) that is sometimes avoidable through additional smarts, it'll always be possible to write two similar programs on either side of that boundary line. If anyone wants to send a PR to move that boundary in the good direction that's fine, but the existence of the boundary itself can't be considered a defect.

@RyanCavanaugh RyanCavanaugh closed this as not planned Won't fix, can't repro, duplicate, stale Jul 29, 2022
@Andarist
Copy link
Contributor Author

Hm, so is it expected that sometimes using an equivalent type through an alias can move the mentioned boundary? The main problem I've seen here is that using a type alias If didn't produce the error when the equivalent "inlined" conditional type did. Is this expected?

You are mentioning that it will always be possible to write two similar programs and land them on either side of that boundary. I agree with that. I'd just like to make a last check if those two programs are considered to be similar because I've considered them to be the same in the same sense that those are the same:

// program1
const result = 2 + 2

// program2
function add(a, b) { return a + b }
const result = add(2, 2)

If extracting parts a more complex type to type aliases (without changing its traits, such as homomorphism, distributivity, etc, in the process) is considered to produce a different/similar program then I'm OK with closing this.

@RyanCavanaugh
Copy link
Member

Hm, so is it expected that sometimes using an equivalent type through an alias can move the mentioned boundary?

Sure, since type aliases can have their variances measured (thus potentially allowing a shortcut measure of the relation between F<T> and F<S> via T -> S), but inline type expressions cannot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not a Defect This behavior is one of several equally-correct options
Projects
None yet
Development

No branches or pull requests

4 participants