Version 1.9.1 Typescript issue with generics

[benneq]

Hello again :D (hope, it doesn't get annoying)

I've written a small function:

const insertAtIndex = <T>(array: T[], index: number, elem: T): T[] => {
    return produce(array, draft => {
        draft.splice(index, 0, elem as any); // <= need "any" for this to work :(
    });
}

Without using as any I get:

Argument of type 'T' is not assignable to parameter of type 'T extends ReadonlyArray<any> ? { [P in keyof T]: Draft<T>; }[keyof T] : DraftObject<T>'. ts(2345)

This error only appears within this generic function in combination with Immer.

These two cases work as expected:

export const insertAtIndex = <T>(array: T[], index: number, elem: T) => {
    array.splice(index, 0, elem);
}

const array = [1,2,3];
const elem = 4;
const newArray = produce(array, draft => {
    draft.splice(42, 0, elem);
})

[aleclarson]

Thank you for finding these edge cases! Need to improve the typescript tests, I guess.

[aleclarson]

Interestingly, when you explicitly type the draft parameter with draft: T[] (which shouldn't be required), a new error appears:

[ts]
Argument of type 'T[]' is not assignable to parameter of type '(this: (draft: T[]) => void, draft: (draft: T[]) => void) => any'.
  Type 'T[]' provides no match for the signature '(this: (draft: T[]) => void, draft: (draft: T[]) => void): any'. [2345]

[benneq]

I changed the code of DraftArray a bit (should still have the same "meaning", right?):
Old:

export type DraftArray<T> = Array<
    T extends ReadonlyArray<any>
        ? {[P in keyof T]: Draft<T>}[keyof T]
        : DraftObject<T>
>

New:

export type DraftArray<T> = T extends ReadonlyArray<any>
    ? Array<{[P in keyof T]: Draft<T>}[keyof T]>
    : Array<DraftObject<T>>

This gives a new error message:

Argument of type 'T' is not assignable to parameter of type 'DraftObject<T>'. ts(2345)

It looks like it chooses the right conditional statement.

And with a last small change to my code (T extends any), the error is gone.
The change to DraftArray is still necessary!
Old:

export const insertAtIndex = <T>(array: T[], index: number, elem: T): T[] => {
    return produce(array, draft => {
        draft.splice(index, 0, elem);
    });
}

New:

                            HERE'S THE CHANGE
                                    |
                                    v
export const insertAtIndex = <T extends any>(array: T[], index: number, elem: T): T[] => {
    return produce(array, draft => {
        draft.splice(index, 0, elem);
    });
}

Of course this change shouldn't be necessary here. Maybe you can figure out how to solve this within Immer.

[aleclarson]

What typescript version are you using? Changing the DraftArray did not change the error for me, but extends any prevents the error as you said.

[benneq]

according to package.json:

"typescript": "3.2.2",

But my tsconfig.json is a bit messy, because it's a legacy project with lot's of js and a bit of ts code. And a mixture of AngularJS (the old one) with decorators, and React (latest version):

{
  "compilerOptions": {
    "target": "es5",
    "module": "commonjs",
    "moduleResolution": "node",
    "sourceMap": true,
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "stripInternal": true,
    "noImplicitAny": false,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": true,
    "noEmitHelpers": true,
    "importHelpers": true,
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "jsx": "react",
    "lib": ["es6", "es2017", "dom", "dom.iterable", "esnext.asynciterable"]
  },
  "exclude": [
    "node_modules",
    "app/_libs",
  ]
}

[aleclarson]

Using typescript 3.1.4, I'm getting [ts] Expected 1-2 arguments, but got 3. [2554] on the draft.splice line. I will try with 3.2.

[aleclarson]

Also you probably want to do ./node_modules/.bin/tsc --version just to be sure your node_modules are up-to-date.

[aleclarson]

Using typescript 3.2.2, using your DraftArray has no effect. :S

[benneq]

I've now tried 3.1.4

There I also get the Expected 1-2 arguments, but got 3. ts(2554) error.

Here's TypeScript's lib.es5.d.ts:
3.1.4:

    splice(start: number, deleteCount?: number): T[];
    splice(start: number, deleteCount: number, ...items: T[]): T[];

3.2.2:

    splice(start: number, deleteCount?: number): T[];
    splice(start: number, deleteCount: number, ...items: T[]): T[];

Looks the same. Maybe it's something from my tsconfig.json :(

Now back at 3.2.2:

$ ./node_modules/.bin/tsc --version
Version 3.2.2

My node_modules is always up to date. I update my packages using rm -rf package-lock.json node_modules && npm i (a bit rough, I know, but this way I'm sure that everything is up2date).

Btw: I didn't check the TypeScript console output. I just looked at VSCode. Let me recheck...
... same behavior. Error is gone with both changes.

I'll record a small video :D

---

EDIT: The video: http://sendvid.com/85i91zpl
First I change the DraftArray definition, then you can see, the error changes in VSCode.
Then I change the insertAtIndex definition. And now the error is gone.

In the console you can see Version: typescript 3.2.2 and at the end: No type errors found.

[aleclarson]

Sorry, I was using a locally-linked version, which had some changes I made for #273. 😅

Still trying to figure this out.. Maybe it's just a bug with Typescript itself?

[benneq]

Yes, maybe it's a TypeScript issue...

I now created a completely blank new TS project.

npm init
npm i --save typescript immer

tsconfig.json

{
  "compilerOptions": {
    "target": "es5",
    "module": "commonjs",
    "lib": ["es6", "es2017"],
    "strict": true,
    "esModuleInterop": true,
  },
  "include": [
    "index.ts"
  ]
}

index.ts

import produce from 'immer';

export const insertAtIndex = <T extends any>(array: ReadonlyArray<T>, index: number, elem: T): ReadonlyArray<T> => {
    return produce(array, draft => {
        draft.splice(index, 0, elem);
    });
}

immer.d.ts

export type DraftArray<T> = T extends ReadonlyArray<any>
    ? Array<{[P in keyof T]: Draft<T>}[keyof T]>
    : Array<DraftObject<T>>

Compile using:

./node_modules/.bin/tsc

Still the same behavior: When changing DraftArray and T extends any it works. Else it doesn't.

---

EDIT:

I now made another change to your TypeScript definitions:

export type DraftObject<T> = T

Note: Even this does not work:

type DraftObject<T> = T extends object ? T : T

With this I don't need T extends any in my code anymore. But this only works when also changing your DraftArray type. Though I'd say, that your DraftArray type should definitely be changed.

The problem seems to be either within DraftObject's {-readonly [P in keyof T]: Draft<T[P]>} or T extends object. Or both? I'm not sure...

---

EDIT 2:

Maybe I found a solution (???)

I only changed DraftArray:

export type DraftArray<T> = T extends ReadonlyArray<any>
    ? Array<{[P in keyof T]: Draft<T>}[keyof T]>
    : T extends object  // T extends AtomicObject, T extends any, T extends object | AtomicObject ... all work !?
    ? Array<DraftObject<T>>
    : Array<T>

Now all errors are gone. But I'm not sure if this is "good".

[aleclarson]

If you want to look into this more, you should..

• fork the next branch
• run yarn && yarn test:dts --watch
• in a new tab, run yarn typed --watch
• finally, try fixing any failed typescript tests by making changes to src/immer.d.ts

[aleclarson]

@benneq Your last change to DraftArray fixes the splice call. But now, try a push call.

[aleclarson]

Alright, I'm counting this as a TypeScript bug.

The best workaround is:

import produce from 'immer';

export function insertAtIndex<T>(array: ReadonlyArray<T>, index: number, elem: T): ReadonlyArray<T>
export function insertAtIndex(array: ReadonlyArray<any>, index: number, elem: any) {
    return produce(array, draft => {
        draft.splice(index, 0, elem);
    });
}

[mweststrate]

@benneq could you setup a small TS based code sandbox (or some other online tool), to fiddle around more easily? I suspect we might be running in a TS limitation here, no be able to expre co- versus contra- variance in generics.

[benneq]

Hi, I just downloaded you next branch. But give me a few minutes, I think I found a fundamental flaw within you Array typings!

Stay tuned!

---

EDIT:

I now searched the internet for "typescript deep immutable" to look how people have done this. (Basically the opposite of "Draft").

Then I tried those code snippets, and most didn't work with generics. But then I found a few that worked. And now things become pretty clear!

In your typings you use T extends any[] or T extends ReadonlyArray<any> and then use something like Array<T[number]>. And I think exactly that is the problem. Because with this approach some type information gets lost.

The working examples for Immutable I found use T extends Array<infer U> or T extends ReadonlyArray<infer U> and then Array<U>.

Now I try to build a small test case and see if it really works...

---

EDIT 2: I don't get it working :(

How about creating an issue within the TypeScript project? But I don't know what to write, because I'm totally unsure where the problem is...

---

EDIT 3: I think I at least now understand the underlying problem...

ReadonlyArray<T> becomes DraftArray<Draft<T>>. And now all those methods on this array won't work anymore with T, but instead need Draft<T>. That's why you can't use splice or push anymore:

export const insertAtIndex = <T>(array: T[], index: number, elem: T): T[] => {
    return produce(array, draft => {
        // "draft" is now something like Array<Draft<T>>
        // though "elem" isn't compatible, because it's of type T, and not of type Draft<T>, even though it's basically the same
        draft.splice(index, 0, elem);
    });
}

---

EDIT 4: In the meantime I found some other way to create recursive models in TypeScript:
microsoft/TypeScript#14174 (comment)
Maybe this helps?

[aleclarson]

@mweststrate The next branch already contains a test for this. So far, the issue seems to be that Draft<T> is not assignable to T.

@benneq I haven't read your diagnosis yet, but that last link looks like a good road to take after v1.9.3.

[benneq]

My diagnonis can be summarized in 1 word: useless :D Yes, maybe that‘s a better approach, but I think it won‘t fix the problem with generics.

…

Am 18.12.2018 um 14:21 schrieb Alec Larson ***@***.***>: @mweststrate The next branch already contains a test for this. So far, the issue seems to be that Draft<T> is not assignable to T. @benneq I haven't read your diagnosis yet, but that last link looks like a good road to take after v1.9.3. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[aleclarson]

Okay, the best workaround (explicit argument types) is demonstrated here:

https://github.com/mweststrate/immer/blob/d60db147173d236727f035fd31e2aac372d7bf8b/__tests__/produce.ts#L151-L163

Here are some related tests:

https://github.com/mweststrate/immer/blob/d60db147173d236727f035fd31e2aac372d7bf8b/__tests__/draft.ts#L291-L300

https://github.com/mweststrate/immer/blob/d60db147173d236727f035fd31e2aac372d7bf8b/__tests__/draft.ts#L19-L27

[aleclarson]

Thanks for the assistance, @benneq! ❤️

[aleclarson]

🎉 This issue has been resolved in version 1.9.3 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[benneq]

Wow. Thank you.
But I‘m pretty sure that your explicit workaround does only work for the first level. I‘ll try your code tomorrow.

But I expect that things like draft[0].splice won‘t work. Or you need a more complex explicit type.

[aleclarson]

@benneq Let me know in a new issue if you hit anything. :)