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

New syntax: (x: X) => mutates x to Y, similar to asserts x as Y but not only narrowing #57273

Closed
6 tasks done
krryan opened this issue Feb 2, 2024 · 4 comments
Closed
6 tasks done
Labels
Duplicate An existing issue was already created

Comments

@krryan
Copy link

krryan commented Feb 2, 2024

🔍 Search Terms

mutation, tuple push

✅ Viability Checklist

Design Goals #5 might be iffy here; I suspect the questions here will be less “how does this work?” and more “is this a good idea?” I think being able to capture a common JavaScript paradigm is worth risking some surprise, but it’s a fair question.

⭐ Suggestion

A special return type similar to (x: X) => asserts x is Y, but instead is mutates x to Y. Does different (less) checking about x and Y than asserts x is Y does, and does not assume that afterwards x is X & Y, instead changing x’s type to just Y.

📃 Motivating Example

The thing that got me started on this is that we were manually creating a FormData structure (for a library):

const form = new FormData();
form.append('foo', 'something');
form.append('foo', 'else');
form.append('bar', 'Coyote Ugly');

And I wanted to type this better, so that form.getAll('foo') knows it’s getting ['something', 'else'] and form.getAll('bar') knows it’s getting ['Coyote Ugly']. I created a TypedFormData interface with a generic parameter Pairs extends Record<string, [FormDataEntryValue, ...FormDataEntryValue[]]>, and used that to type all of the methods. For mutating methods, I used asserts this is to indicate the new value.

For example, to type append (with a string value, I had overloads for blobs and blobs-with-filenames), I used

export interface TypedFormData<Pairs extends Record<string, [FormDataEntryValue, ...FormDataEntryValue[]]> = {}> extends FormData {
    append<K extends string, V extends string>(name: K, value: V): asserts this is TypedFormData<{
        [P in (keyof Pairs) | K]: [
            ...(P extends keyof Pairs ? Pairs[K] : []),
            ...(P extends K ? [V] : [])
        ] extends infer NewTuple extends [FormDataEntryValue, ...FormDataEntryValue[]] ? NewTuple : never
    }>;

This actually works—once. That is, after form.append('foo', 'something') in my above, t has type TypedFormData<{ foo: ['something'] }> and all my typing works. However, further calls to append fail: I get TypedFormData<{ foo: ['something'] }> & TypedFormData<{ foo: ['something', 'else'] }> and then that does not behave correctly with any my method signatures to indicate the correct type.

This is due to how asserts works, and makes sense because asserts wasn’t meant for this scenario. But I would like something that is.

💻 Use Cases

  1. Mutating objects in-place is a pretty common JavaScript pattern. There’s a trend towards immutable objects and not doing this kind of thing—which is great—but there’s a ton of existing code and libraries, including lots of the standard library, that mutate like this. There is a lot of use-case here.

  2. The obvious concern is that it is arguably extremely surprising for something annotated as one thing to have a different type later. Assertions—narrowing—are one thing, since it still fits the original annotation, but this would be something actually changing and quite possibly no longer fitting that annotation at all.

    Another issue is more practical: one of the biggest use-cases for this is (non-readonly) tuples, and this suggestion alone doesn’t solve all of the problems for those. All of the mutating methods on Array also return values (e.g. push returning the new length), which is a problem here—and arguably something that also needs to be considered for asserts x is Y constructions, because a function that asserts a type may also return things. That’s a separate concern but without addressing both, one of the strongest use-cases for this change (tuples) isn’t actually going to be solved by this change.

  3. The “workaround,” such as it is, is to just accept that you can’t generate an interface that will track its own type this way. You either set the type to something extremely broad and generic—losing any type safety you might have had by being more specific—or you literally just accept types that are wrong or use type casting to tell TS what is going on. We have the signatures of native methods being completely wrong in certain cases, such as the issues with mutating (non-readonly) tuples in Bug: incorrect tuple (array) type after changing in place #52375. Per Remove destructive methods in tuple type #6325, TS isn’t going to restrict non-readonly tuples to use ReadonlyArray methods, so this is a fairly-serious instance of unsound-ness that isn’t going away.

@MartinJohns
Copy link
Contributor

Essentially a duplicate of #22865.

@krryan
Copy link
Author

krryan commented Feb 2, 2024

I saw that, and it is the same concept, but a different syntax more in line with what Typescript already has (at this point).

@MartinJohns
Copy link
Contributor

A different syntax for the very same problem usually does not warrant a new issue. It's better suited as a comment.

Quoting RyanCavanaugh:

we prefer to organize issues by use case / end-goal, not have competing separate issues for different syntactic representations.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Feb 2, 2024
@typescript-bot
Copy link
Collaborator

This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants