Fix the AsyncResult.andThen handling of known-Ok transformations #189
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
Without these overloads
goodResult.andThen((value) => Promise.resolve(Ok(value * 2)));
was of type
AsyncResult<uknown, string | number>
which would cause downstream problems.
Granted, this could be an edge case and the map method could be better
for this (Ok -> Ok transformation so only the value inside changes) but
still, I don't think we should be generating unknown here, hence the patch.
Resolves: #176
jstasiak
requested review from
a team,
rbruggem,
raguiar9080,
charcarr,
brittanydanishevsky,
0bex0 and
jasharifestina
jstasiak
changed the title
jstasiak
added a commit
that referenced
this pull request
Feb 10, 2025
This patch reverts commit [1] and documents it.
The problem is after [1] the following code
const fromRegularResult = (Ok(1) as Result<number, string>).toAsyncResult();
fromRegularResult.andThen((value) => Ok(value * 2));
would failt co compile:
src/file.ts:169:19 - error TS2349: This expression is not callable.
Each member of the union type '{ <T2>(mapper: (val: never) => Ok<T2> | Promise<Ok<T2>> | AsyncResult<T2, never>): AsyncResult<T2, string>; <T2, E2>(mapper: (val: never) => Result<...> | ... 1 more ... | AsyncResult<...>): AsyncResult<...>; } | { ...; }' has signatures, but none of those signatures are compatible with each other.
169 fromRegularResult.andThen((value) => Ok(value * 2));
~~~~~~~
src/file.ts:169:28 - error TS7006: Parameter 'value' implicitly has an 'any' type.
169 fromRegularResult.andThen((value) => Ok(value * 2));
~~~~~
I spend the last Friday's afternoon trying to figure out why and how to
fix it and I failed. I think reverting the original patch is the way to
go for now and it was handling an edge anyway.
It's mort important that andThen works in general cases (where the
callbacks return Result, not always Ok).
[1] 9259f83 ("Fix the AsyncResult.andThen handling of known-Ok transformations (#189)")
jstasiak
added a commit
that referenced
this pull request
Feb 10, 2025
This patch reverts commit [1] and documents this fact.
The problem is after [1] the following code
const fromRegularResult = (Ok(1) as Result<number, string>).toAsyncResult();
fromRegularResult.andThen((value) => Ok(value * 2));
would fail co compile:
src/file.ts:169:19 - error TS2349: This expression is not callable.
Each member of the union type '{ <T2>(mapper: (val: never) => Ok<T2> | Promise<Ok<T2>> | AsyncResult<T2, never>): AsyncResult<T2, string>; <T2, E2>(mapper: (val: never) => Result<...> | ... 1 more ... | AsyncResult<...>): AsyncResult<...>; } | { ...; }' has signatures, but none of those signatures are compatible with each other.
169 fromRegularResult.andThen((value) => Ok(value * 2));
~~~~~~~
src/file.ts:169:28 - error TS7006: Parameter 'value' implicitly has an 'any' type.
169 fromRegularResult.andThen((value) => Ok(value * 2));
~~~~~
I spent the last Friday's afternoon trying to figure out why and how to
fix it and I failed. I think reverting the original patch is the way to
go for now and it was handling an edge case anyway.
It's more important that andThen works in general cases (where the
callbacks return Result, not always Ok).
[1] 9259f83 ("Fix the AsyncResult.andThen handling of known-Ok transformations (#189)")
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.
Without these overloads
was of type
which would cause downstream problems.
Granted, this could be an edge case and the map method could be better for this (Ok -> Ok transformation so only the value inside changes) but still, I don't think we should be generating unknown here, hence the patch.
Resolves: #176