Skip to content

Commit

Permalink
fix(react,vue,solid,svelte): reset to previous onError and throw if s…
Browse files Browse the repository at this point in the history
…uspense (#305)
  • Loading branch information
nmathew98 committed Apr 16, 2024
1 parent bb94a15 commit 5f2e70d
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 22 deletions.
2 changes: 1 addition & 1 deletion packages/react/src/test/use-qwery/mutations.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ describe("mutations", async () => {
expect(store.get("test")).toEqual({
a: 1,
});
expect(rerenders).toBe(2); // 2x on mount
expect(rerenders).toBe(3); // 1x on mount, 1x dispatch, 1x on error
});
});

Expand Down
1 change: 1 addition & 0 deletions packages/react/vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ export default defineConfig({
typecheck: {
enabled: true,
},
dangerouslyIgnoreUnhandledErrors: true,
},
});
44 changes: 35 additions & 9 deletions packages/shared/src/use-qwery/create-qwery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,19 @@ export const createQwery = ({
});
}

// If `onChange` rejects and `suspense`, then create a new Promise
// throwing the error here
// Here specifically because it helps with debugging not to throw
// it at `onError`. If we throw it at `onError` the stacktrace shows
// `incremental` instead
if (suspense && result instanceof Promise) {
result.catch(reason => {
throw reason;
});

return result;
}

if (!result) {
if (queryKey && fetchPolicy !== "network-only") {
cache?.makeOnChange?.(queryKey)(args[0]);
Expand All @@ -55,14 +68,6 @@ export const createQwery = ({
rerender();
}

if (suspense && result instanceof Promise) {
return (result as Promise<unknown>).catch(error => {
onError?.(args[0], args[1]);

throw error;
});
}

return result;
},
});
Expand Down Expand Up @@ -97,11 +102,32 @@ export const createQwery = ({
},
});

const proxiedOnError = new Proxy(onError, {
apply: (onError, thisArg, args) => {
const previous = args[1];

Reflect.apply(onError, thisArg, args);

if (queryKey && broadcast) {
channel?.postMessage({
id,
next: previous,
});
}

if (queryKey && fetchPolicy !== "network-only") {
cache?.makeOnChange?.(queryKey)(previous);
}

rerender();
},
});

const crdt = createCRDT({
initialValue: initialValue,
onChange: proxiedOnChange,
onSuccess: proxiedOnSuccess,
onError: onError,
onError: proxiedOnError,
trackVersions: debug,
});

Expand Down
16 changes: 7 additions & 9 deletions packages/solid/src/test/use-qwery/async.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,23 @@ describe("async", async () => {

const _render = () =>
render(() => (
<ErrorBoundary fallback={<p>Error</p>}>
<Suspense fallback="Loading...">
<Suspense fallback="Loading...">
<ErrorBoundary fallback={err => <span>{err.message}</span>}>
<AppAsync
initialValue={initialValue}
onChange={vitest.fn()}
render={data => data() && data?.a}
/>
</Suspense>
</ErrorBoundary>
</ErrorBoundary>
</Suspense>
));

expect(_render).not.toThrowError();

expect(screen.getByText("Loading...")).toBeTruthy();

await waitFor(() => {
expect(screen.getByText("Error")).toBeTruthy();
expect(screen.getByText("Error!")).toBeTruthy();
});

await waitFor(() => {
Expand All @@ -85,16 +85,14 @@ describe("async", async () => {
});

it("shows fallback if `onChange` rejects", async () => {
const onChange = vitest.fn(async () => {
throw new Error("Test!");
});
const onChange = vitest.fn(() => Promise.reject("Test!"));
const initialValue = vitest.fn(() => [1]);

let dispatchQwery;
const _render = () =>
render(() => (
<Suspense fallback="Loading...">
<ErrorBoundary fallback={<span>Error!</span>}>
<ErrorBoundary fallback={err => <span>{err}</span>}>
<AppAsync
initialValue={initialValue}
onChange={onChange}
Expand Down
2 changes: 1 addition & 1 deletion packages/solid/src/test/use-qwery/mutations.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ describe("mutations", async () => {
expect(store.get("test")).toEqual({
a: 1,
});
expect(rerenders).toBe(2); // 2x on mount
expect(rerenders).toBe(3); // 1x on mount, 1x dispatch, 1x on error
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,5 @@ test("invokes-onerror-if-onchange-rejects", async () => {
expect(store.get("test")).toEqual({
a: 1,
});
expect(props.options.exports.rerenders).toBe(2); // 1x on mount
expect(props.options.exports.rerenders).toBe(3); // 1x on mount, 1x dispatch, 1x on error
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,5 @@ test("invokes-onerror-if-onchange-rejects", async () => {
expect(store.get("test")).toEqual({
a: 1,
});
expect(props.exports.rerenders).toBe(1); // 1x on mount
expect(props.exports.rerenders).toBe(1); // TODO: should be 3 (1x on mount, 1x dispatch, 1x on error)?
});

0 comments on commit 5f2e70d

Please sign in to comment.