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

fix: incorrect return type for async produceWithPatches #933

Merged
merged 1 commit into from May 11, 2022
Merged

fix: incorrect return type for async produceWithPatches #933

merged 1 commit into from May 11, 2022

Conversation

mylesj
Copy link
Contributor

@mylesj mylesj commented May 5, 2022

👋 Hello, it looks to be a long standing definition but this line seems to be incorrect.

For reference:

Screenshot 2022-05-05 at 01 50 10

@netlify
Copy link

netlify bot commented May 5, 2022

Deploy Preview for quizzical-lovelace-dcbd6a canceled.

Name Link
🔨 Latest commit c91a842
🔍 Latest deploy log https://app.netlify.com/sites/quizzical-lovelace-dcbd6a/deploys/62731e76f8a8dd000869e24e

mylesj added a commit to mylesj/immer-compose that referenced this pull request May 5, 2022
@coveralls
Copy link

Pull Request Test Coverage Report for Build 2273093401

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 98.553%

Totals Coverage Status
Change from base Build 2257376231: 0.0%
Covered Lines: 786
Relevant Lines: 794

💛 - Coveralls

@mweststrate mweststrate merged commit 9f7623d into immerjs:master May 11, 2022
@mweststrate
Copy link
Collaborator

Nice catch, thanks!

@github-actions
Copy link
Contributor

🎉 This PR is included in version 9.0.13 🎉

The release is available on:

Your semantic-release bot 📦🚀

@weihomechen
Copy link

weihomechen commented Dec 25, 2023

Has async produceWithPatches been removed in v10 ?

I can't use like this:

 produceWithPatches({a: 0}, async d => {
        await Promise.resolve()
        d.a = 1
 }).then(([nextState, patches, inversePathes]) => {
 }))

@mylesj
Copy link
Contributor Author

mylesj commented Dec 26, 2023

@weihomechen Looks like a yes,

Drop promise based producer support. Immer supports returning a promise from a producer. I don't think I ever saw usage of that in the wild, and it is a bit of an oxymoron as it makes it easy to leak drafts. Since producers should be like enter-write-exit without happening anything in the mean time, and it complicates the code base unnecessarily, I'm proposing to simply drop it.

@weihomechen
Copy link

weihomechen commented Jan 22, 2024

@weihomechen Looks like a yes,

Drop promise based producer support. Immer supports returning a promise from a producer. I don't think I ever saw usage of that in the wild, and it is a bit of an oxymoron as it makes it easy to leak drafts. Since producers should be like enter-write-exit without happening anything in the mean time, and it complicates the code base unnecessarily, I'm proposing to simply drop it.

I have to say it's a bad 'news' ...
but thanks @mylesj anyway

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants