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

Limit of 25 members when using union of mapped type #40803

Closed
fluggo opened this issue Sep 28, 2020 · 15 comments
Closed

Limit of 25 members when using union of mapped type #40803

fluggo opened this issue Sep 28, 2020 · 15 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@fluggo
Copy link

fluggo commented Sep 28, 2020

TypeScript Version: 4.0.2

Search Terms: 25, limit, mappings, mapped, label:"Domain: Big Unions", keyof

Expected behavior: Unions of mapped types (or keyof said mapped type) should work through assignment

Actual behavior: Compiler raises error when one keyof parameter is assigned to another keyof parameter. Removing any of the 26 entries in the mapped type solves the issue.

Related Issues: #35493 is similar; this question on StackOverflow appears to deal with the same behavior

Code

// Commenting out any of these fields fixes the error
type AWSObjectPayloads = {
  'ec2/vpc': { a: number },
  'ec2/image': { b: number },
  'ec2/subnet': { c: number },
  'ec2/security-group': { d: number },
  'ec2/instance': { e: number },
  'ec2/prefix-list': { f: number },
  'ec2/network-interface': { g: number },
  'ec2/vpc-endpoint': { h: number },
  'ec2/nat-gateway': { i: number },
  'ec2/dhcp-options': { j: number },
  'ec2/elastic-ip': { k: number },
  'ec2/flow-log': { l: number },
  'ec2/internet-gateway': { m: number },
  'ec2/network-acl': { n: number },
  'ec2/peering-connection': { o: number },
  'ec2/route-table': { p: number },
  'ec2/vpn-gateway': { q: number },
  'elb/load-balancer': { r: number },
  'elb-v2/load-balancer': { s: number },
  'elb-v2/target-group': { t: number },
  'rds/subnet-group': { u: number },
  'rds/cluster': { v: number },
  'rds/instance': { w: number },
  'redshift/subnet-group': { x: number },
  'redshift/cluster': { y: number },
  'cloudtrail/trail': { z: number },
};

interface AWSObjectImpl<Name, Desc> {
  objectType: Name;
  desc: Desc;
}

type AWSObjectTypesByName = {
  [K in keyof AWSObjectPayloads]: AWSObjectImpl<K, AWSObjectPayloads[K]>
};

type AWSObjectTypeName = keyof AWSObjectTypesByName;
type AWSObject<T extends AWSObjectTypeName = AWSObjectTypeName> = AWSObjectTypesByName[T];

// Using this definition for AWSObject corrects the assignment error, but
// eliminates the type narrowing:
//
//   type AWSObject<T extends AWSObjectTypeName = AWSObjectTypeName> = AWSObjectImpl<T, AWSObjectPayloads[T]>;

function foo(test: AWSObjectTypeName) {
  const testA: AWSObject = {
    objectType: test,
    desc: null as any,
  };

  if(testA.objectType === 'ec2/vpc')
    console.log(testA.desc.a);
}
Output
"use strict";
// Using this definition for AWSObject corrects the assignment error, but
// eliminates the type narrowing:
//
//   type AWSObject<T extends AWSObjectTypeName = AWSObjectTypeName> = AWSObjectImpl<T, AWSObjectPayloads[T]>;
function foo(test) {
    const testA = {
        objectType: test,
        desc: null,
    };
    if (testA.objectType === 'ec2/vpc')
        console.log(testA.desc.a);
}
Compiler Options
{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "alwaysStrict": true,
    "esModuleInterop": true,
    "declaration": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "moduleResolution": 2,
    "target": "ES2017",
    "jsx": "React",
    "module": "ESNext"
  }
}

Playground Link: Provided

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Sep 28, 2020
@RyanCavanaugh
Copy link
Member

When the source object doesn't have any it's pretty straightforward to demonstrate that this kind of initialization is rarely sound, but we try to make this particular kind of assignment work anyway by enumerating all possible values that the initializer could have, and seeing if that value would be assignable to AWSObject. There's a hardcoded limit here of 24 candidates (more typically hit when you have e.g. 2 x 3 x 3 possible values) for this process to avoid combinatorial explosions.

@fluggo
Copy link
Author

fluggo commented Sep 28, 2020

I don't know what you mean by your first sentence. Was there a better way to achieve this same effect?

I can work around this error by casting the test parameter to "any" first, and that's no major loss, but if I'm running into a design limitation, I sure would like a way to do this correctly.

@etlovett
Copy link

Here's another example of this limit of 25 union entries causing a false type error: #42518 / Playground.

@NoriSte
Copy link

NoriSte commented Feb 12, 2021

Here another example: Gist - Playground.

Thanks

@elias6
Copy link

elias6 commented Mar 7, 2023

This is a bit annoying and confusing. Is there any way to increase or remove this limit?

@zaquille-oneil
Copy link

instead of having the typescript error "malfunction", can we at least have it indicate that there is some sort of a limit? We had only found out about this limit by commenting out 1 mapped type at a time until the error message went away.

@Pablo1107
Copy link

@RyanCavanaugh what was the reason this issue got closed in 2021? The project I'm working on it's using TypeScript v5.0.4 (released 2023-04-07) and this error it's still happening.

If there's a technical limitation that makes this issue difficult to resolve, at least there should be a clearer error than the current behaviour is showing.

TypeScript Error
│          Type '{ comm_details: { comm_type: "string1" | "string2" | "string3" | "string4" | ... 22 more ... | "string5"; }; ...' is not assignable to type '{ comm_details: { ...; } | ... 78 more ... | { ...; }; } & { ...; }'. 
│            Type '{ comm_details: { comm_type: "string1" | "string2" | "string3" | "string4" | ... 22 more ... | "string5"; }; ...' is not assignable to type '{ comm_details: { ...; } | ... 78 more ... | { ...; }; }'. 
│              Types of property 'comm_details' are incompatible. 
│                Type '{ comm_type: "string1" | "string2" | "string3" | "string4" | ... 22 more ... | "string5"; }' is not assignable to type '{ comm_type: "string6"; } | { comm_type: "string7"; } | ... 77 more ... | { ...; }'. 
│                  Type '{ comm_type: "string1" | "string2" | "string3" | "string4" | ... 22 more ... | "string5"; }' is not assignable to type '{ comm_type: "string9"; }'. 
│                    Types of property 'comm_type' are incompatible. 
│                      Type '"string1" | "string2" | "string3" | "string4" | "string8" | ... 21 more ... | "string5"' is not assignable to type '"string9"'. 
│                        Type '"string1"' is not assignable to type '"string9"'. 

I write here because it's the oldest issue posted with this behavior. But I'm aware of this response.

@RyanCavanaugh
Copy link
Member

There's no upper limit on how much work could be incurred here; you could have an object with 16 properties each with 16 possible values; do you expect TS to do the full expansion of 18,446,744,073,709,551,616 different possible values? Certainly not. So there has to be an arbitrary limit somewhere; the limit we chose is 24 because that's always going to be reasonably fast.

@Pablo1107
Copy link

There's no upper limit on how much work could be incurred here; you could have an object with 16 properties each with 16 possible values; do you expect TS to do the full expansion of 18,446,744,073,709,551,616 different possible values? Certainly not. So there has to be an arbitrary limit somewhere; the limit we chose is 24 because that's always going to be reasonably fast.

And what about improving the error message showed when this happens? Is it possible or even reasonable to expect that a better error message should be shown? I imagine something along the lines of:

Txxx: cannot compare more than 24 members of an union of type x

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Apr 16, 2024

It's extremely common for this limiter to hit and for nothing undesired to happen as a result. If we knew that the non-assignability only occurred because we hadn't expanded the union, we would have simply not reported that it was non-assignable in the first place.

What you're describing would imply that you would see that message in the majority of cases where the source type was a nontrivial union, even though only a tiny fraction of the time would a full expansion have meant a successful assignment

@zaquille-oneil
Copy link

What you're describing would imply that you would see that message in the majority of cases where the source type was a nontrivial union, even though only a tiny fraction of the time would a full expansion have meant a successful assignment

Maybe I'm understanding this incorrectly, but I think most commenters in this thread would feel that something like a warning message shown when the union exceeds 25 would be extremely helpful instead of the current error being shown.

Perhaps "Warning: exceeding 24 members can not be accurately compared"? it would be more honest than what's being shown - is this possible, desirable? could this possibly be achieved via a configurable option?

@RyanCavanaugh
Copy link
Member

I don't think you're understanding it correctly, or have a very different perspective on whether or not TypeScript's error messages have enough noise in them. The only way we could do this would be to issue the error every time the source union was reasonably big, which is an incredibly common occurrence.

In effect, nearly every time you saw an assignability error, it would say "We couldn't expand it because it was too big", and ~99% of the time, expanding the union wouldn't have caused the assignment to succeed anyway.

@mitchazj
Copy link

is a flag for increasing the limit a little above 25 a possibility? eg, to 30, or 100

this flag would be really helpful for people who

  • know what they're doing (ie they understand why the limit exists), and
  • are willing to tradeoff a little bit of performance to make their code work?

Thinking for example of large teams who have critical code blocked by this limitation, and who need to either buy themselves some time to re-architecture or just swallow a small performance hit (costs of having a huge codebase etc)

@Pablo1107
Copy link

is a flag for increasing the limit a little above 25 a possibility? eg, to 30, or 100

this flag would be really helpful for people who

* know what they're doing (ie they understand why the limit exists), and

* are willing to tradeoff a little bit of performance to make their code work?

Thinking for example of large teams who have critical code blocked by this limitation, and who need to either buy themselves some time to re-architecture or just swallow a small performance hit (costs of having a huge codebase etc)

Well, in my case I change my code to compare in batches of unions to avoid having one union of more than 25 items. Maybe this is better than increasing the limit performance-wise?

@jozefchutka
Copy link

After spending hours on detective work on my complex code base, I also ended up here. I just wonder how many dev-hours has been wasted on this design limitation which at the same time is extremely common for this limiter to hit, yet there is no interest to resolve this limitation or even provide reasonable error message.

My case is as follows:

type Kind = 1 | 2 | 3 ... 26;
type Option<T extends Kind> = {kind:T;}
type Options = Option<1> | Option<2> | Option<3> ... Option<26>; // Option<Kind> is not suitable in my case
const kind:Kind = getKind(...);
const result:Options = {kind};

Type '{ kind: Kind; }' is not assignable to type 'Options'.
  Type '{ kind: Kind; }' is not assignable to type 'Option<26>'.
    Types of property 'kind' are incompatible.
      Type 'Kind' is not assignable to type '26'.
        Type '1' is not assignable to type '26'.

I was looking around and try to figure out any workaround whatsoever, just to figure out that such "design is wrong", but no examples of good design, and no workarounds.

The workaround I eventually came up with is something as silly as:

const result:Options = kind === 1 || kind === 2 ? {kind} : {kind};

I hope some more people can share their ideas on good design or workaround

rhystmills added a commit to guardian/prosemirror-elements that referenced this issue Aug 15, 2024
…lements having a length greater than 24. I've taken one deprecated

element out so that we come under the limit.

We should prune this list anyway, having a set of elements that demonstrate features of the library (e.g. field types), rather than
specific elements used in Composer.

Related issue: microsoft/TypeScript#40803

Co-authored-by: Jonathon Herbert <jonathon.herbert@guardian.co.uk>
rhystmills added a commit to guardian/prosemirror-elements that referenced this issue Aug 20, 2024
…all text fields (#424)

* Add plugin which adds widget decorations around the word 'widget' anywhere it appears, so that we can test widget decorations in isolation within the demo

* Add identifier to test widget decoration

* Remove existing decoration fix related to repeater depth, because it didn't solve the decoration bug in all cases. Add new fix, which explicitly handles DecorationSet
and DecorationGroup, either of which we could receive. Make sure the decorations are offset correctly, in a manner that supports Widget and Node decorations in addtiion
to inline decorations

* Improve comment explaining the class declaration for DecorationGroup

* Tidy widget decoration code in demo helpers

* Apply linter

* Restore some code that didn't need to be changed

* Add tests and supporting code from Jon's branch

Co-authored-by: Jonathon Herbert <jonathon.herbert@guardian.co.uk>

* Solve TypeScript build error. This was related to the union type of elements having a length greater than 24. I've taken one deprecated
element out so that we come under the limit.

We should prune this list anyway, having a set of elements that demonstrate features of the library (e.g. field types), rather than
specific elements used in Composer.

Related issue: microsoft/TypeScript#40803

Co-authored-by: Jonathon Herbert <jonathon.herbert@guardian.co.uk>

* Add tests for widget decorations specifically

* Add test for case where decorations are dropped if they exist in an element following existing document content

* Remove unnecessary code

Co-authored-by: Jonathon Herbert <jonathon.herbert@guardian.co.uk>

* Remove unnecessary code

Co-authored-by: Jonathon Herbert <jonathon.herbert@guardian.co.uk>

* Remove unnecessary code

Co-authored-by: Jonathon Herbert <jonathon.herbert@guardian.co.uk>

* Recreate decorations in context of inner editor's node, to avoid losing inline decorations

* Move DecorationGroup to declaration file

* Move decoration handling functions to own file as pure functions

* Remove this.setDecorationsForEditor, which is now redundant

* Add changeset

---------

Co-authored-by: Jonathon Herbert <jonathon.herbert@guardian.co.uk>
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

No branches or pull requests

9 participants