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

Intellisense String Unions autocomplete inconsistent with ts #49996

Closed
dereekb opened this issue Jul 22, 2022 · 11 comments
Closed

Intellisense String Unions autocomplete inconsistent with ts #49996

dereekb opened this issue Jul 22, 2022 · 11 comments
Labels
Not a Defect This behavior is one of several equally-correct options

Comments

@dereekb
Copy link

dereekb commented Jul 22, 2022

Bug Report

🔎 Search Terms

Intellisense
String Unions
Incorrect Autocomplete
Types
Typing

🕗 Version & Regression Information

This bug seems like it has been around since the introduction of String Unions.

The following example is of a type that will merge the keys of the input object into an ascending order (which as I write this bug template I notice it is not lexicographical order), since that's how Typescript uses the individual types in the unions of types. This ordering is desirable since it is predictable. Without this order, the following StringConcatenationOrder type would be unusable, as would the other related types.

⏯ Playground Link

export declare type UnionToIntersection<U> = (U extends any ? (k: U) => void : never) extends (k: infer I) => void
  ? I
  : never;

export type UnionToOvlds<U> = UnionToIntersection<U extends any ? (f: U) => void : never>;
export type PopUnion<U> = UnionToOvlds<U> extends (a: infer A) => void ? A : never;

export type StringConcatenationOrder<S extends string, SEPARATOR extends string> = PopUnion<S> extends infer SELF
  ? //
    SELF extends string
    ? Exclude<S, SELF> extends never
      ? `${SELF}`
      : // This works because the values of S are always interpreted in ascending lexiographical order
        `${StringConcatenationOrder<Exclude<S, SELF>, SEPARATOR>}${SEPARATOR}${SELF}`
    : never
  : never;

export type KeyCanBeString<K> = K extends number | boolean | string | null | undefined ? K : never;

export type OrderedCommaSeparatedKeysOfObject<T extends object> = OrderedCommaSeparatedKeys<`${KeyCanBeString<keyof T>}`>;
export type OrderedCommaSeparatedKeys<T extends string> = StringConcatenationOrder<T, ','>;

// @showEmit
const object = {
  _: 0,
  b: 0,
  a: 0,
  c: 0,
  1: 0
};

const replaced: OrderedCommaSeparatedKeysOfObject<typeof object> = '_,a,b,c,1';

Workbench Repro

🙁 Actual behavior

Screen Shot 2022-07-22 at 1 28 41 AM

In VS Code Intellisense looks at the object's variable declaration order (_, b, a, c, 1), but ts will look at the ascending "lexicographical" order of the object keys (_,a,b,c,1).

In the workspace you can see it give an error, but ts compiles it. This throws me off sometimes since intellisense will present invalid completions that typescript will reject (correctly). Here's the ts error thrown by Jest:

Screen Shot 2022-07-22 at 2 41 52 AM

🙂 Expected behavior

Intellisense should evaluate the object's keys in the ascending order consistent with ts.

@dereekb
Copy link
Author

dereekb commented Jul 22, 2022

I noticed my CI failed with a case that shows that ts isn't consistent either with how it handles union of string keys.

Screen Shot 2022-07-22 at 3 07 28 AM

I assumed the variables came back in a consistent order but I suppose they don't. Ideally I would expect the keys to be in some order, but it's possible that the UnionToIntersection type and inferences do weird things.

In any case, having a consistent result between ts and Intellisense is necessary. At the end of the day for this type all I'm aiming for is to make sure the string has the expected comma-separated values.

@fatcerberus
Copy link

The order of types in a union is essentially arbitrary (based on the internal ID numbers of the types involved); type unions should generally be treated as unordered sets. Consistency between TS and IntelliSense is a moot point as the observed order of union constituents may change just by changing your code, or even just by renaming files so they get compiled in a different order. Any code that relies on a specific union order is indeed going to be very fragile, as you've observed here.

All may not be lost however - see #17944.

@MartinJohns
Copy link
Contributor

Since I see the type here again, FYI: #41857 (comment)

We don't support anything that involves UnionToIntersection; this is an ill-defined and will frequently produce "surprising" results.

@dereekb
Copy link
Author

dereekb commented Jul 22, 2022

Thanks for the responses. I found another response while digging through the similar threads.

#13298 (comment)

It seems like until there is a type that can sort the input union of keys it can't be safely achieved with the typings provided above.

My main need for these types is to allow any combination of the input and combine them with a separator (comma):

 'a' | 'b' | 'c' w/ ',' -> 'a' | 'b' | 'c' | 'a,b' | 'b,a' | 'c,a,b' | etc...

And any combination of the input that uses each key once and combine them with a separator (comma):

'a' | 'b' | 'c' w/ ',' -> 'a,b,c' | 'a,c,b' | 'b,a,c' | etc...

I need an unordered set that is interpreted in an ordered fashion. 'B' | 'C' | 'A' should behave the same a as 'B' | 'A' | 'C', but it needs to be consistent when evaluated. I think #32224 is the proposed fix that is referenced.

The use case example is here:

...

export type ProfileModelCrudFunctionsConfig = {
  profile: {
    update: {
      _: UpdateProfileParams;
      username: SetProfileUsernameParams;
      onboard: [FinishOnboardingProfileParams, boolean];
    };
    delete: UpdateProfileParams;
  };
  profilePrivate: null;
};

export const profileModelCrudFunctionsConfig: ModelFirebaseCrudFunctionConfigMap<ProfileModelCrudFunctionsConfig, ProfileTypes> = {
  profile: ['update:_,username,onboard', 'delete']
};

...

ProfileModelCrudFunctionsConfig is a type I use as a map to enforce type safety, but it is unavailable at runtime. I use the config object to generate the corresponding interface as an object at runtime:

/**
 * Declared as an abstract class so we can inject it into our Angular app using this token.
 */
export abstract class ProfileFunctions implements ModelFirebaseFunctionMap<ProfileFunctionTypeMap, ProfileModelCrudFunctionsConfig> {
  abstract [profileSetUsernameKey]: FirebaseFunctionMapFunction<ProfileFunctionTypeMap, 'profileSetUsername'>;
  abstract profile: {
    updateProfile: {
      // full names
      updateProfile: ModelFirebaseCrudFunction<UpdateProfileParams>;
      updateProfileUsername: ModelFirebaseCrudFunction<SetProfileUsernameParams>;
      updateProfileOnboard: ModelFirebaseCrudFunction<FinishOnboardingProfileParams, boolean>;
      // short names
      update: ModelFirebaseCrudFunction<UpdateProfileParams>;
      username: ModelFirebaseCrudFunction<SetProfileUsernameParams>;
      onboard: ModelFirebaseCrudFunction<FinishOnboardingProfileParams, boolean>;
    };
    deleteProfile: ModelFirebaseCrudFunction<UpdateProfileParams>;
  };
}

I need to make sure that _, username, and onboard are in that string so that the function that uses the config can correctly split the input string and create the above functions (updateProfile, updateProfileUsername, etc.) at runtime. Without type safety it is left to the runtime errors to catch any missing values in that comma separated string.

What I have works most of the time, but the more items in the combination, the greater the chance of Typescript and Intellisense not being on the same page and one of them having an error.

As a workaround for now I can just drop in any and silence the issue after I've picked the string that Intellisense or Typescript want me to use, but then I lose the type safety that I value greatly for an important and type sensitive area of code.

That said, the fact that the typing system is flexible enough to do this in Typescript is crazy. Really appreciate the work you guys do and the quick responses.

@MartinJohns
Copy link
Contributor

My main need for these types is to allow any combination of the input and combine them with a separator (comma):

Given the limit of 25 types in a union this sounds... suboptimal.

@dereekb
Copy link
Author

dereekb commented Jul 22, 2022

My main need for these types is to allow any combination of the input and combine them with a separator (comma):

Given the limit of 25 types in a union this sounds... suboptimal.

You're right, the combinations aren't great, since would create n! of type combinations. Since the typings bail out around 2^16, that means the max number of types we could have would be up to n=8, but n=7 is the highest that I'd even bother recommending due to speed. I was exploring it as an option since it would successfully reproduce every combination for each type in the union. I should have been a little more clear, it's not really an end-need but rather a "need" to overcome the current limitations; not really a need at all.

For the example above I really just need to ensure each string is in there, which means if the keys were ordered consistently through each variation, I could rely on the following:

'a' | 'c' | 'b' w/ ',' -> 'a,b,c'

But since Typescript doesn't order the keys in the union in any particular order during evaluation, I can't rely on that so I need to "cast a net" right now via combinations to get lucky enough to catch a value that the different interpreters consistently hit.

I have another function that creates an "approximation" that contains each entry that is in the order that typescript ordered the types and it works the best right now in terms of flexibility for the inconsistencies.

As for the 25 union limit itself, it's not a concern for what I'm using it for. It looks like there's a newer limit of >40 that would be more than enough.

export declare type UnionToIntersection&lt;U> = (U extends any ? (k: U) => void : never) extends (k: infer I) => void
  ? I
  : never;

export type UnionToOvlds&lt;U> = UnionToIntersection&lt;U extends any ? (f: U) => void : never>;
export type PopUnion&lt;U> = UnionToOvlds&lt;U> extends (a: infer A) => void ? A : never;

export type StringConcatenationOrder&lt;S extends string, SEPARATOR extends string> = PopUnion&lt;S> extends infer SELF
  ? //
    SELF extends string
    ? Exclude&lt;S, SELF> extends never
      ? `${SELF}`
      : // This works because the values of S are always interpreted in ascending lexiographical order
        `${StringConcatenationOrder&lt;Exclude&lt;S, SELF>, SEPARATOR>}${SEPARATOR}${SELF}`
    : never
  : never;

export type KeyCanBeString&lt;K> = K extends number | boolean | string | null | undefined ? K : never;

export type OrderedCommaSeparatedKeysOfObject&lt;T extends object> = OrderedCommaSeparatedKeys&lt;`${KeyCanBeString&lt;keyof T>}`>;
export type OrderedCommaSeparatedKeys&lt;T extends string> = StringConcatenationOrder&lt;T, ','>;

// @showEmit
const object = {
  a: 0,
  b: 0,
  c: 0,
  d: 0,
  e: 0,
  f: 0,
  g: 0,
  h: 0,
  i: 0,
  j: 0,
  k: 0,
  l: 0,
  m: 0,
  n: 0,
  o: 0,
  p: 0,
  q: 0,
  r: 0,
  s: 0,
  t: 0,
  u: 0,
  v: 0,
  w: 0,
  x: 0,
  y: 0,
  z: 0,
  0: 0,
  1: 0,
  2: 0,
  3: 0,
  4: 0,
  5: 0,
  6: 0,
  7: 0,
  8: 0,
  9: 0,
  10: 0,
  11: 0,
  12: 0,
  13: 0,
  14: 0,
  15: 0,
  16: 0,
  17: 0,
  18: 0,
  19: 0,
  20: 0,
  21: 0
};

const replaced: OrderedCommaSeparatedKeysOfObject&lt;typeof object> = 'a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21'

Workbench Repro

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

Ordering of a union is not supposed to be observable from a type system perspective, and if you find yourself observing it, you've wandered off the trail and need to find your way back 😅

#17944 will not fix this because that would only be plausibly be addressed during the type-to-string process; internal ordering would remain unchanged for performance reasons.

@dereekb
Copy link
Author

dereekb commented Jul 29, 2022

That makes sense to me.

I guess this is out of scope for the time being. I'd put in a feature request for adding some Template Literal Types that can merge the union and compare it unordered but since the existing types are all mappings of types it might not be doable right now because this is a joining of types.

https://www.typescriptlang.org/docs/handbook/2/template-literal-types.html

This can be represented currently:

Object.keys(passedObject).map(x => `${x}Changed`)

This cannot:

Object.keys(passedObject).join(',');

Even if implemented for type-checking performance it isn't feasible since it's not trivial to know if a certain string matches any of the possible combinations without actually calculating them all, and working backwards isn't predictable either if a type contained the separator.

My feature request would instead be to support the equivalent type of:

Object.keys(passedObject).sort().join(',');

I'd be open to creating a PR for it if you believe it would be feasible to implement and is inline with Typescript's design goals.

@RyanCavanaugh
Copy link
Member

This seems like an XY problem; what are you trying to do?

@dereekb
Copy link
Author

dereekb commented Jul 31, 2022

Example use case in more detail is above/here:

#49996 (comment)

I have keys on a type (ProfileModelCrudFunctionsConfig above) I want to make sure are all comma-separated in a string, so that if I add any fields the type system provides additional guarding. The proper string is needed so I can generate an object that fits the ProfileFunctions interface at runtime.

I'd like a type that is the equivalent of Object.keys(...).sort().join(','); so the typing system can assert that the string contains each of the types (_, username, onboard in the example).

So if I added "newCrudFunction" there, it reminds me to add it to the string.

Screen Shot 2022-07-31 at 12 37 06 AM

@github-actions
Copy link

github-actions bot commented Jun 8, 2023

This issue has been marked as 'Not a Defect' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 8, 2023
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