immer 3.0

[mweststrate]

• Implement Remove support for this = draft in recipes #308: no this
• Implement Idea: Ignore return value of producer #246, ignore return value, use return replace(value) for replacement
• Merge Improve Flow types #232: improved flow typings
• Improve minification by using property mangling?
• Drop support for Node 6

[coveralls]

Coverage remained the same at 99.726% when pulling 1f056dc on immer3 into 42b14bd on master.

[aleclarson]

As I've said in #246, I'm highly against ignoring the return value by default. The current behavior is much better, and I think the void trick should be discouraged in the docs, since it's just a footgun for no real gain. If anything, the return value should always be used so the mental model is closer to a traditional reducer. In the end, there's really not a strong enough justification for changing the current behavior at all.

[aleclarson]

I'll review this today, too. (New York timezone)

Can you reword the commit messages to fit the Conventional Commits style?

[aleclarson]

Also, I'm a little worried about the changes to the flow types, considering this comment. Maybe it would be best to leave the flow types alone, or remove them entirely?

[migueloller]

@aleclarson, I agree with your sentiment. The general intent of the changes I proposed are in the right direction, I think, but unless a newer version of Flow fixed a few things there were some issues with those Flow types.

This is nothing that can't be tested, though. So if all the Flow tests are passing, and we should make sure they're not false positives, it should be alright. Perhaps adding some more Flow tests could help as well?

If this is a change that you guys really want to get in for v3 maybe I can set aside some time to look into it this week.

[mweststrate]

@migueloller, ah misunderstood the latest commits, thought that they addressed the open issues. Will revert this change (we can also merge it in later).

I peeked in more detail at your changes, at least they are partially incorrect: produce (uncurried) always produces S, not R! Same for the curried function, that also produces S. The recipe itself can indeed, in both cases, either return S | void.

[mweststrate]

@aleclarson sorry, I really should make that a habit! Rebasing...

[mweststrate]

eh lol, I'll try again

[mweststrate]

Note: build is currently not being executed due to the repo transfer. It didn't sync to travis yet....

[migueloller]

@mweststrate,

I peeked in more detail at your changes, at least they are partially incorrect: produce (uncurried) always produces S, not R! Same for the curried function, that also produces S. The recipe itself can indeed, in both cases, either return S | void.

That is correct. I made a mistake when writing these types to assume the issue I was trying to fix was because the return type of the recipe was S | void when really the issue was that because the type of a Redux reducer is (RS | void, A) => RS, immer was inferring S as RS | void.

I think the solution to this problem is proper use of type variance.

[aleclarson]

I wonder if we should test typescript@next too (or instead), for early heads up?

[mweststrate]

I think I'll revert it to 3.4, some hardcoded versions. TS is quite aggressive with their latest tag, and having random build failures is annoying. It is not the CI's responsibility to test compatible with random TS versions and gives us heads ups. The point is to make sure we didn't break anything ourselves

[aleclarson]

LGTM! 👍

I simplified the IProduce type tests (see here). :)

[mweststrate]

@aleclarson. Lol, thanks for doing the obvious thing :)

[aleclarson]

Whoops ;)

[aleclarson]

I cancelled the deploy for you 👍

[mweststrate]

Lol, something isn't working as supossed to. Hopefully release works fine on next build, otherwise, could you trigger the 3.0 release?

…

On Wed, Apr 17, 2019 at 4:31 PM Alec Larson ***@***.***> wrote: 🎉 This PR is included in version 2.2.0 🎉 The release is available on: - npm package ***@***.*** dist-tag) <https://www.npmjs.com/package/immer> - GitHub release <https://github.com/immerjs/immer/releases/tag/v2.2.0> Your *semantic-release <https://github.com/semantic-release/semantic-release>* bot 📦🚀 — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#309 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhCMJ1xIFhtwcAVIq_8ljum_lIe8Uks5vhzBbgaJpZM4agJ-v> .

[aleclarson]

@mweststrate Did you see my review comment about the migration guide?

[mweststrate]

@aleclarson ehh nope. I have to leave currently. Would you mind triggering a 3.0 release, and updating migration guide if needed? I didn't figure out how to trigger a major release with semantic-release yet.

[aleclarson]

🎉 This PR is included in version 3.0.0 🎉

The release is available on:

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

Your semantic-release bot 📦🚀

[aleclarson]

The BREAKING CHANGE part goes in the footer 😛

[mweststrate]

Argh Op wo 17 apr. 2019 17:17 schreef Alec Larson <[email protected]>:

…

The BREAKING CHANGE part goes in the footer 😛 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#309 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhPln-NuZYvipNf58NOjeoBjIFPabks5vhzr5gaJpZM4agJ-v> .