createAsyncThunk: serializeError option #812
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Also pinging @nathggns on that one. If you really see any value in this, try to convince me :) |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 32281aa:
|
phryneas
force-pushed
the
asyncThunk-serialize-throw
branch
from
94c67a2
to
c6346cf
Compare
phryneas
changed the title
phryneas
force-pushed
the
asyncThunk-serialize-throw
branch
4 times, most recently
from
ca13a71
to
02c8e04
Compare
markerikson
approved these changes
Nov 27, 2020
.github/workflows/tests.yml
Outdated
| - name: Prefix `freeze` re-export for pre-3.7 TS versions with @ts-ignore | ||
| if: ${{ matrix.ts < 3.7 }} | ||
| run: | | ||
| sed -i -e "/export .* freeze .* from 'immer'/s/^/\/\/ @ts-ignore\n/" dist/typings.d.ts |
phryneas
force-pushed
the
asyncThunk-serialize-throw
branch
from
02c8e04
to
32281aa
Compare
msutkowski
reviewed
Nov 27, 2020
| - name: Prefix `freeze` re-export for pre-3.7 TS versions with @ts-ignore | ||
| if: ${{ matrix.ts < 3.7 }} | ||
| run: | | ||
| sed -i -e "/import .* freeze .* from 'immer'/s/^/\/\/ @ts-ignore\n/" dist/typings.d.ts |
|
Thanks for this — sorry I couldn't get around to writing this myself after our conversations. I think this is going to make quite a big difference to removing a lot of complexity in some of our code and I appreciate it. |
|
Should |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This would address two requests from #792:
serializeErroroption tocreateAsyncThunkawait dispatch(async(3, { unwrap: true }))and return it with the unwrapped result and otherwise throw an error. So, essentially an inlined version ofunwrapResult.I say would because I'm still not a fan of Nr 2. Even less, after writing all this, tbh.
The amount of code (and especially types) required to support a second way of doing something we already have in the lib makes me uneasy.
Yes, it might be slightly "nicer". But only slightly. And it's a lot of code. And a lot of new behaviour (especially type-wise) that we have to commit to keep running.
So my personal suggestion would be to keep 1. (even though it also does something comparable to
rejectWithValueas well) and not merge 2 (no matter the amount of work I put into this).