Override unwrap behavior for buildInitiateQuery, update tests #1786
Conversation
|
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 c8221fe:
|
size-limit report 📦
|
| subscriptionOptions, | ||
| queryCacheKey, | ||
| abort, | ||
| unwrap, | ||
| unwrap() { | ||
| return aggregatePromise.then<any>((result) => { | ||
| if (result.isError) { | ||
| throw result.error | ||
| } | ||
|
|
||
| return result.data | ||
| }) | ||
| }, |
There was a problem hiding this comment.
The only thing that I think that could be interesting (or confusing?) here is what happens regarding unwrap and abort. If abort is called, the initial request will be aborted, but a consumer will still be able to unwrap this. At the same time, I wouldn't want the abort to ignore the aggregate result, if that makes sense. I don't think there is any real issue with this implementation, but it could be confusing. I don't have any better ideas or solutions for this though.
There was a problem hiding this comment.
.abort of the originally running promise should set the query into an error state, so I think this would result in an error here, which seems reasonable.
You bring up another problem though: .abort() on this would do nothing if it just piggy-backs on another promise that started earlier. But otoh, I guess that's okay, too - you would not want to abort something else by accident I guess?
There was a problem hiding this comment.
Can we make this function async? The .then usage here irritates me for some reason 😅
| subscriptionOptions, | ||
| queryCacheKey, | ||
| abort, | ||
| unwrap, | ||
| unwrap() { | ||
| return aggregatePromise.then<any>((result) => { | ||
| if (result.isError) { | ||
| throw result.error | ||
| } | ||
|
|
||
| return result.data | ||
| }) | ||
| }, |
There was a problem hiding this comment.
.abort of the originally running promise should set the query into an error state, so I think this would result in an error here, which seems reasonable.
You bring up another problem though: .abort() on this would do nothing if it just piggy-backs on another promise that started earlier. But otoh, I guess that's okay, too - you would not want to abort something else by accident I guess?
| subscriptionOptions, | ||
| queryCacheKey, | ||
| abort, | ||
| unwrap, | ||
| unwrap() { | ||
| return aggregatePromise.then<any>((result) => { | ||
| if (result.isError) { | ||
| throw result.error | ||
| } | ||
|
|
||
| return result.data | ||
| }) | ||
| }, |
There was a problem hiding this comment.
Can we make this function async? The .then usage here irritates me for some reason 😅
Co-authored-by: Lenz Weber <mail@lenzw.de>
Co-authored-by: Lenz Weber <mail@lenzw.de>
| fireEvent.click(fetchButton) // This technically dispatches a ConditionError, but we don't want to see that here. We want the real result to resolve and ignore the error. | ||
|
|
||
| await waitFor(() => { | ||
| const dataResult = screen.getByTestId('error')?.textContent |
There was a problem hiding this comment.
should be
const dataResult = screen.getByTestId('result')?.textContent
This is an attempt at solving the issue where if you execute multiple lazy queries and unwrap them, anything after the first would throw with a
ConditionErroras requests 2+ would becondition: falsedue to a request being in flight.