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

TypeScript 3.4.0 support #325

Closed
wants to merge 3 commits into from
Closed

TypeScript 3.4.0 support #325

wants to merge 3 commits into from

Conversation

aleclarson
Copy link
Member

Noticed an issue with the Immutable<T> type while testing out typescript@3.4.0-dev.20190306.

This PR will also track other Immer-related issues with TypeScript 3.4.0 until 3.4.0 is officially released.

@coveralls
Copy link

coveralls commented Mar 7, 2019

Coverage Status

Coverage remained the same at 99.746% when pulling d8d7a1d on ts-3.4 into e592b5c on master.

@aleclarson
Copy link
Member Author

aleclarson commented Mar 30, 2019

We need a way to run TypeScript tests based on the installed version, since there are breaking changes between TypeScript 3.4 and 3.3. Here are the solutions I can think of:

  1. During each CI build, apply a git patch to our tests and install the TypeScript version associated with said patch. This could be done for multiple TypeScript versions with breaking changes.

  2. Drop support for TypeScript 3.3.x and under

  3. Find another way... 😅

Thoughts, @mweststrate?

@aleclarson
Copy link
Member Author

aleclarson commented Mar 30, 2019

Oh, I forgot about typesVersions introduced in TypeScript 3.1:
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-1.html#version-selection-with-typesversions

But we'll still need to git-patch the tests, I think.

@aleclarson
Copy link
Member Author

This PR is dependent on prettier/prettier#5977 being fixed.

@aleclarson
Copy link
Member Author

During each CI build, apply a git patch to our tests and install the TypeScript version associated with said patch. This could be done for multiple TypeScript versions with breaking changes.

It would be nice if we could only run the TypeScript tests when we know the typedefs have changed, but I don't think anyone has solved that yet. Otherwise, CI builds are slowed down by needing to install multiple versions of TypeScript to ensure safe upgrades / backward-compatible changes.

@jineshshah36
Copy link

jineshshah36 commented Apr 5, 2019

I'm getting Type instantiation is excessively deep and possibly infinite. pretty much in every producer on 3.4. I found that making the following change fixed it:

// old
// export type Draft<T> = T extends never[]
//   ? T
//   : T extends ReadonlyArray<any>
//   ? T[number][] extends T
//     ? DraftArray<T>
//     : DraftTuple<T>
//   : T extends AtomicObject
//   ? T
//   : T extends object
//   ? { -readonly [P in keyof T]: Draft<T[P]> }
//   : T

// new
export type Draft<T> = T extends AtomicObject
  ? T
  : T extends object
  ? { -readonly [K in keyof T]: Draft<T[K]> }
  : T

Since 3.4 improves support for readonly arrays and tuples, this seems to work without breaking anything.

@brunolemos
Copy link

brunolemos commented Apr 10, 2019

Please release a new version with typescript 3.4 support, the latest stable vscode already comes with this version.

I'm also getting Type instantiation is excessively deep and possibly infinite. on every immer producer. Tried to use typescript 3.4.3, had to downgrade.

Current workaround for users

Replace the immer Draft type definition to something as simple as this:

export type Draft<T> = T

You can replace it editing the node_modules file, using patch-package or maybe create a .d.ts with declare module 'immer' { (haven't tried this way).

@jineshshah36
Copy link

@brunolemos that is not a valid workaround. At a minimum, you need something like the following in order to correctly type the draft:

export type Draft<T> = T extends AtomicObject
  ? T
  : T extends object
  ? { -readonly [K in keyof T]: Draft<T[K]> }
  : T

The reason is if T has any readonly properties, they need to be erased

@mweststrate
Copy link
Collaborator

I found some tricks to better handle the recursion in TS types, will investigate later whether this solves the open issues.

@mweststrate
Copy link
Collaborator

Lol, it was already in there. Awesome test setup btw @aleclarson!

@mweststrate
Copy link
Collaborator

closing in favor of #348 (which was based branched from this PR)

@aleclarson aleclarson deleted the ts-3.4 branch April 18, 2019 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants