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

Proposal/Feature-Request: add "indexed assignment types" T[assign K] #41363

Open
5 tasks done
Nathan-Fenner opened this issue Nov 2, 2020 · 0 comments
Open
5 tasks done
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@Nathan-Fenner
Copy link
Contributor

Search Terms

  • indexed type assign

Related: #30769 "improve soundness of indexed access types"

This would be a simpler/nicer solution to #41233.

Suggestion

A new typing syntax, proposed here as T[assign K] analogous to the existing indexed type T[K], which resolves to a type of allowable values which could be assigned to obj[k] when obj: T and k: K. Specifically, when T is a union, this will require performing an intersection of possible value types, rather than a union.

Use Cases

Consider the following example functions:

function updateProperty<T, K extends keyof T>(obj: T, key: K, val: T[K]): void {
   obj[key] = val;
}

function copyWithNewProperty<T, K extends keyof T>(obj: T, key: K, val: T[K]): T {
    return { ...obj, [key]: val };
}

Both functions have been given reasonable types, and these functions type-check today.

However, their types are subtly wrong when T is a union type. For example,

type Shape = { shape: "square", sides:  4 } | { shape: "polygon", sides: number };

function makeShape(): Shape {
  return { shape: "square", sides: 4 };
}
const square: Shape = makeShape();
updateProperty(square, "sides", 7);

The call to updateProperty will change square's sides property to be the value 7, even though this shouldn't be allowed. That's because today, TypeScript is a little bit unsound when it comes to assigning properties in generic functions. This proposal doesn't recommend changing that behavior, since that would likely affect a lot of code that's already written.

The reason for this behavior is that Shape["sides"] will distribute over the union in Shape's definition, producing 4 | number which just becomes number.

What we would like to happen is that instead it becomes an intersection over all possibilities; since those are the values that are safe to possibly assign: 4 & number which should simplify to 4. But we don't want this to happen in all cases - the existing indexed type T[K] still has many uses. Instead we want to create a new type syntax T[assign K] that performs the correct transformation.

It is currently possible for a library author to implement a version of this using the "intersection to union hack", but the types involved are complex and the result is verbose, making it difficult to actually incorporate. In addition, it's less obvious that this is possible, let alone necessary, to provide sound types to your library's users.

But adding the new "indexed assignment types", we obtain:

function updateProperty<T, K extends keyof T>(obj: T, key: K, val: T[assign K]): void {
   obj[key] = val;
}

function copyWithNewProperty<T, K extends keyof T>(obj: T, key: K, val: T[assign K]): T {
    return { ...obj, [key]: val };
}

With these new types, the function call updateProperty(square, "sides", 7) would fail to type-check, since 7 is not assignable to 4 & number. On the other hand, updateProperty(square, "sides", 4) would continue to type-check.

Future Soundness Improvements

Once the "indexed assignment type" exists in the language, TypeScript could add a strictness flag (e.g. --strictGenericIndexAssignment) that ensures indexed types are used soundly in generic code. Thus the original implementation for updateProperty and copyWithNewProperty could be identified as problematic and rejected with a diagnostic (something along the lines of T[K] is not assignable to T[assign K] with some custom help text to explain how this can be a soundness problem).

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code

To support this syntax, assign must be made into a contextual keyword within indexed types. A simple rule would be the following: assign is treated as a keyword if the next (non-space) token could be the start of a type (e.g. it's an identifier foo, string/number literal "hello"/5, opening curly brace {, open parentheses () and not an operator (e.g. assign | keyof Z could parse today, so we want to avoid changing the behavior of that code).

There are several cases of ambiguity surrounding arrays and tuple types. Depending on the prevalence of assign as a type identifier specifically appearing in indexed types, these cases may or may not require additional thought.

For example, T [ assign [ Q ] ] could be interpreted either as "T indexed by the type assign[Q] (current interpretation)" or as "T assign-indexed by the type [Q]". Note that the latter interpretation is nonsensical, at least today, since types cannot be indexed by tuples.

Similarly, T [ assign [ Q ] extends A ? Y : N ] could be interpreted either as "T indexed by assign[Q] extends A ? Y : N (current interpretation)" or "T assign-indexed by [Q] extends A ? Y : N". In this case, the latter interpretation is actually plausible.

Because of the ambiguity, I think it would be better to keep the first interpretation and to warn/reject/lint both examples, and instead encourage wrapping the ambiguity index type in parentheses, as in T[assign ([Q] extends A ? Y : N)].

  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@Nathan-Fenner Nathan-Fenner changed the title Add "indexed assignment types" T[assign K] Proposal/Feature-Request: add "indexed assignment types" T[assign K] Nov 2, 2020
@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

2 participants