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

Circular reference error when defining Record type #41164

Closed
jp7837 opened this issue Oct 19, 2020 · 20 comments · May be fixed by #57293
Closed

Circular reference error when defining Record type #41164

jp7837 opened this issue Oct 19, 2020 · 20 comments · May be fixed by #57293
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@jp7837
Copy link

jp7837 commented Oct 19, 2020

TypeScript Version: 4.0.2 (and 4.1.0-dev.20201015)

Search Terms: circularly, Record

Code

type T1 = { [key: string]: T1 }
type T2 = Record<string, T2>

Expected behavior:
No compilation errors

Actual behavior:
While type T1 compiles properly, type T2 reports Type alias 'T2' circularly references itself. even though they are inherently the same type. This was actually found by autofixing a new @typescript-eslint rule consistent-indexed-object-style

Playground Link: https://www.typescriptlang.org/play?ts=4.0.2#code/C4TwDgpgBAKgjFAvFA3lA2gawiAXFAZ2ACcBLAOwHMBdfeKAXwFgAoUSWAJiSgCUIAxgHtiAEwA8RMlQA0XAHxA

Related Issues: None found

@RyanCavanaugh
Copy link
Member

Certain circularities are allowed (e.g. T1) but other circularities aren't, e.g.

type Identity<T> = T;
type T3 = Identity<T3>;

Generic instantiation is deferred, so at the point TS analyzes T2's declaration, it can't tell if it's in the Record case (would be allowed) or the Identity case (would not be allowed). It's only later in the process that we could tell that this is actually OK, but if it wasn't OK, it's "too late" to go back and start complaining about it.

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Oct 19, 2020
@jp7837
Copy link
Author

jp7837 commented Oct 19, 2020

@RyanCavanaugh thanks for the explanation. Not that it's TypeScript team's responsibility, but just as an FYI this may become a more popular issue as teams use the consistent-indexed-object-style rule from @typescript-eslint. I can file a bug on their project to not auto-fix this case if you think that's a good idea.

@gbegher
Copy link

gbegher commented Oct 21, 2020

@RyanCavanaugh The behaviour seems a bit inconsistent to me (or at least not very transparent)

In the following example a code change somewhere deep down resulted in a failure.

Playground link

namespace Works {
    type Value = Sum<{
        PRODUCT: Product<Value>
    }>

    // Defined in another file

    type Product<T = any> = Record<string, T>;

    type ValueOf<X> = X[keyof X];
    type Sum<Cases extends Product> = ValueOf<SumMapping<Cases>>;
    type SumMapping<Cases extends Product> = {
        [cas in keyof Cases]:
            // ------ difference is here ---------
            {
                type: cas;
                value: Cases[cas];
            };
    };

    
}

namespace Fails {
    type Value = Sum<{
        PRODUCT: Product<Value>
    }>

    // Defined in another file

    type Product<T = any> = Record<string, T>;

    type ValueOf<X> = X[keyof X];
    type Sum<Cases extends Product> = ValueOf<SumMapping<Cases>>;
    type SumMapping<Cases extends Product> = {
        [cas in keyof Cases]:
            // ------ difference is here ---------
            [type: cas, value: Cases[cas]]
    };
}

@nscarcella
Copy link

@RyanCavanaugh not sure what's under the hood of Record, but it does feel a bit inconsistent that some generics seem to work fine with recursions while others don't:

type A<T> = T | Array<A<T>>  // This line works fine
type R = Record<string, R>      // this one doesn't

@jedwards1211
Copy link

jedwards1211 commented Nov 18, 2021

@RyanCavanaugh

at the point TS analyzes T2's declaration, it can't tell if it's in the Record case (would be allowed) or the Identity case (would not be allowed)

I don't understand how what you're saying applies to OP? Because OP doesn't have an Identity type alias, only your example does.

@jedwards1211
Copy link

jedwards1211 commented Nov 18, 2021

I guess what you're saying is that a type alias can't reference itself if it's used in a type parameter of an ordinary generic type instantiation:

type MyRecord<V> = {[key: string]: V}

type Stuff = MyRecord<string | Stuff> // circular reference error

I don't get why this is technically impossible to support though. Let's say we're checking that {a: {b: 'c'}} is of type Stuff.

My layperson imagination of how that could work is:

  1. Required type gets expanded to {[key: string]: string | Stuff}
  2. Check that keys of {a: {b: 'c'}} are of type string (ok)
  3. check that values of {a: {b: 'c'}} are of type string | Stuff
    a. check that {b: 'c'} is of type string | Stuff
    1. It's obviously not a string, see if it's of type Stuff
    2. Required type Stuff gets expanded to {[key: string]: string | Stuff}
    3. check that keys of {b: 'c'} are of type string (ok)
    4. check that values of {b: 'c'} are of type string | Stuff
      a. check that 'c' is of type string | Stuff
      b. 'c' is of type string (ok)

Additionally, Flow supports the Stuff type just fine, so it seems perfectly possible. Or does it lead to other inconsistent behavior in Flow that I'm unaware of? Or are there some other niche cases in TS that are fundamentally incompatible with supporting the Stuff type?

@jedwards1211
Copy link

jedwards1211 commented Nov 18, 2021

To add to what @nscarcella was saying, Array has some internal difference from an ordinary generic type alias that isn't transparent enough, because this equivalent type alias doesn't work:

type A<T> = T | Array<A<T>> // okay

type MyArray<T> = T[]

type B<T> = T | MyArray<B<T>> // errors

@jedwards1211
Copy link

jedwards1211 commented Nov 18, 2021

Actually, I was wrong about Flow, it fails to produce expected errors on a similar recursive generic type alias:

// @flow
type Record<K, V> = {[K]: V}
type Stuff<T> = Record<string, T | Stuff<T>>
const foo: Stuff<number> = { a: 'not a number' } // should error, but doesn't

https://flow.org/try/#0PTAEAEDMBsHsHcBQAXAngBwKagEqYMawBOAJgDwDSANKAGoB8oAvKAN4DaFAugFx0C+KDNgDKyAK6RIZACqMWeQqTIBnZEQCWAOwDmNGaAA+oMZOlz6iQlrWhIsWH1NSyW8QFsARpiLy2oAEM+AHItWGRA0DcvH2DQflAQUBUAC1hxaBJQHyJiGk9xCJJYTBUtYOREIA

Maybe there is something fundamentally hard about supporting such types...I wish I had a more concrete understanding of why, because mathematically speaking, this type definition seems perfectly coherent

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Nov 18, 2021

Imagine I told you that an integer was a flarpnumber if it was either prime or one of its digits was a flarpnumber. Is 6 a flarpnumber?

@jedwards1211
Copy link

jedwards1211 commented Nov 18, 2021

@RyanCavanaugh I mean obviously that seems undecidable, but I still don't see what it has to do with OP's example of type T2 = Record<string, T2>? Whether something like {foo: {}} is of type T2 seems perfectly decidable (not sure about deciding subtype/supertype relationships but that must not be a problem when T2 is defined as T2 = {[key: string]: T2}, so alias or not, the actual structure we're talking about is well-defined)

@jedwards1211
Copy link

jedwards1211 commented Nov 18, 2021

The thing that's frustrating about this is if we define

type MyRecord<V> = {[key: string]: V}

Because of the = the natural expectation is that both sides are truly equivalent and interchangeable. But this equivalence gets violated when we take

type T = {[key: string]: T} // okay

And then try to substitute the RHS with what should be equivalent:

type T = MyRecord<T> // error, even though this means the same thing

So MyRecord<V> isn't actually equivalent to {[key: string]: V} in all cases. Are there other cases I don't know about where the sides of a type alias aren't equivalent?

@jp7837
Copy link
Author

jp7837 commented Jan 12, 2022

BTW, the issue I mentioned with typescript-eslint was fixed (typescript-eslint/typescript-eslint#2687)

@Pyrolistical
Copy link

@jedwards1211 good point, also what you've described is a workaround for this very issue!

type ThisFails = number | Record<string, ThisFails>

type ThisWorks = number | {[key: string]: ThisWorks}

playground

@FStefanni
Copy link

Hi,

@jedwards1211 good point, also what you've described is a workaround for this very issue!

type ThisFails = number | Record<string, ThisFails>

type ThisWorks = number | {[key: string]: ThisWorks}

playground

yes, I was expecting this to work... but it does not :(
I hope in a quick improvement.

Regards

@yw662
Copy link

yw662 commented Dec 12, 2022

This seems work:

// eslint-disable-next-line @typescript-eslint/no-empty-interface
interface ThisWorks extends Record<string, ThisWorks> {}

{ [key: string]: T } may not be very useful since { 'foo': 'bar' } is a Record<string, string> but not a { [key: string]: T }

@1hko
Copy link

1hko commented Feb 12, 2023

Why is it so painful to write recursive programs with TypeScript?

type texpr =
  | number
  | string
  | Array<texpr>
  | Promise<texpr>
  | Record<string, texpr> // circular reference error 😓
async function evaluate(t: texpr): Promise<texpr> {
  switch (t.constructor) {
    case Number: ...
    case String: ...
    case Array: ...
    case Object: ...
    case Promise: ...
  }
}

What's the fix without a bunch of goofy business?

@Pyrolistical
Copy link

Hi,

@jedwards1211 good point, also what you've described is a workaround for this very issue!

type ThisFails = number | Record<string, ThisFails>

type ThisWorks = number | {[key: string]: ThisWorks}

playground

yes, I was expecting this to work... but it does not :( I hope in a quick improvement.

Regards

What do you mean it doesn't work? It works for me. The first one errors the second one doesn't.

@FStefanni
Copy link

@Pyrolistical: exactly, the first one does not work
Regards

@TyIsI
Copy link

TyIsI commented Aug 3, 2023

My apologies for replying here. But figured I would double check before creating a possible issue.

Extending the playground @Pyrolistical posted above, I get an error while trying to delete a subkey.

image

type ThisFails = number | Record<string, ThisFails>;

type ThisWorks = number | { [key: string]: ThisWorks };

type ButThisErrors =
  | { [key: string]: ButThisErrors | string | number }
  | string
  | number;

const test1TopLevel: ButThisErrors = {
  l2: { l3: { l4: { l5: { l6: "endkey" } } } },
};

delete test1TopLevel.l2.l3.l4.l5.l6;

interface AndSoDoesThis {
    [key: string]: AndSoDoesThis | string | number
}
const test2TopLevel: AndSoDoesThis = {
  l2: { l3: { l4: { l5: { l6: "endkey" } } } },
};

delete test2TopLevel.l2.l3.l4.l5.l6;

Playground

I'm guessing that this is due to the cut-off for recursive parsing?

@2colours
Copy link

2colours commented May 2, 2024

Why is this closed as completed? Where is the solution and what is the version where it works as expected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.