Skip to content

Commit

Permalink
Rewrote deferred internals to wrap a and implement the full API rathe…
Browse files Browse the repository at this point in the history
…r than .
  • Loading branch information
bvaughn committed Mar 8, 2023
1 parent 71f5762 commit b50d819
Show file tree
Hide file tree
Showing 10 changed files with 158 additions and 143 deletions.
3 changes: 3 additions & 0 deletions packages/suspense/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Changelog

## 0.0.23
* Rewrote deferred internals to wrap a `Promise` and implement the full `Promise` API rather than `PromiseLike`.

## 0.0.22
* Fixed type definition for internal `ResolvedRecordData` structure.

Expand Down
16 changes: 10 additions & 6 deletions packages/suspense/src/cache/createCache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,9 @@ describe("createCache", () => {
});

it("it should throw if value was rejected", async () => {
cache.readAsync("error-expected");
await Promise.resolve();
try {
await cache.readAsync("error-expected");
} catch (error) {}
expect(() => cache.getValue("error-expected")).toThrow("error-expected");
});
});
Expand All @@ -315,9 +316,10 @@ describe("createCache", () => {
});

it("should return undefined for values that have rejected", async () => {
cache.readAsync("error");
await Promise.resolve();
expect(cache.getValueIfCached("async")).toBeUndefined();
try {
await cache.readAsync("error-expected");
} catch (error) {}
expect(cache.getValueIfCached("error-expected")).toBeUndefined();
});
});

Expand Down Expand Up @@ -454,7 +456,9 @@ describe("createCache", () => {

it("should return the correct value for keys that have already been resolved or rejected", async () => {
cache.readAsync("async");
cache.readAsync("error");
try {
await cache.readAsync("error");
} catch (error) {}

await Promise.resolve();

Expand Down
10 changes: 9 additions & 1 deletion packages/suspense/src/cache/createCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,15 @@ export function createCache<Params extends Array<any>, Value>(
function prefetch(...params: Params): void {
debugLogInDev(`prefetch()`, params);

readAsync(...params);
const promiseOrValue = readAsync(...params);
if (isPromiseLike(promiseOrValue)) {
promiseOrValue.then(
() => {},
(error) => {
// Don't let readAsync throw an uncaught error.
}
);
}
}

function readAsync(...params: Params): PromiseLike<Value> | Value {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -359,9 +359,6 @@ describe("createIntervalCache", () => {
deferred.reject(new Error("Expected"));
await expect(() => promise).rejects.toThrow("Expected");

// Wait for promise rejection to finish
await Promise.resolve();

expect(() => cache.readAsync(1, 5, "test")).toThrow(
"Cannot load interval"
);
Expand Down Expand Up @@ -418,14 +415,18 @@ describe("createIntervalCache", () => {
return deferredRejects;
}
});
let promise = cache.readAsync(5, 9, "test");
cache.readAsync(5, 9, "test");
deferredResolves.resolve([5, 6]);

deferredRejects.reject(new Error("Expected"));
await expect(() => promise).rejects.toThrow("Expected");

// The failed interval will fail future requests that contain it
await expect(cache.readAsync(5, 9, "test")).rejects.toThrow("Expected");
await expect(cache.readAsync(9, 9, "test")).rejects.toThrow("Expected");
await expect(() => cache.readAsync(5, 9, "test")).rejects.toThrow(
"Expected"
);
await expect(() => cache.readAsync(9, 9, "test")).rejects.toThrow(
"Expected"
);

// Evicting the failed interval will allow it to be requested again
cache.evictAll();
Expand Down
13 changes: 13 additions & 0 deletions packages/suspense/src/cache/createStreamingCache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,12 @@ describe("createStreamingCache", () => {
it("notifies subscriber(s) of progress and rejection", async () => {
const streaming = cache.stream("string");

// Prevent Jest from failing due to unhandled promise rejection
streaming.resolver.then(
() => {},
() => {}
);

expect(fetch).toHaveBeenCalledTimes(1);
expect(fetch.mock.lastCall?.[1]).toEqual("string");

Expand All @@ -293,6 +299,7 @@ describe("createStreamingCache", () => {
expect(streaming.value).toEqual([1]);

options.reject("Expected");

expect(subscription).toHaveBeenCalledTimes(2);
expect(streaming.complete).toEqual(true);
expect(streaming.progress).toBeUndefined();
Expand All @@ -313,6 +320,12 @@ describe("createStreamingCache", () => {

const streaming = cache.stream("string");

// Prevent Jest from failing due to unhandled promise rejection
streaming.resolver.then(
() => {},
() => {}
);

expect(fetch).toHaveBeenCalledTimes(1);
expect(fetch.mock.lastCall?.[1]).toEqual("string");

Expand Down
2 changes: 1 addition & 1 deletion packages/suspense/src/hooks/useCacheMutation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ describe("useCacheMutation", () => {
console.error = () => {};

await act(async () => {
mutationApi.two.mutateAsync(["two"], async () => {
await mutationApi.two.mutateAsync(["two"], async () => {
throw "errored-two";
});
});
Expand Down
9 changes: 7 additions & 2 deletions packages/suspense/src/hooks/useCacheMutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,12 @@ export function useCacheMutation<Params extends Array<any>, Value>(
status: STATUS_REJECTED,
};

deferred.reject(error);
try {
deferred.reject(error);
await deferred;
} catch (error) {
// Don't trigger an unhandled rejection
}

backupRecordMap.set(cacheKey, record);

Expand All @@ -154,7 +159,7 @@ export function useCacheMutation<Params extends Array<any>, Value>(
});
} finally {
// Cleanup after mutation by deleting the abort controller
// If this mutation has already been aborted by a condata mutation
// If this mutation has already been preempted by a newer mutation
// don't delete the newer controller
if (abortController === mutationAbortControllerMap.get(cacheKey)) {
mutationAbortControllerMap.delete(cacheKey);
Expand Down
98 changes: 62 additions & 36 deletions packages/suspense/src/hooks/useImperativeCacheValue.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* @jest-environment jsdom
*/

import { Component, PropsWithChildren } from "react";
import { createRoot } from "react-dom/client";
import { act } from "react-dom/test-utils";
import { createCache } from "../cache/createCache";
Expand All @@ -13,6 +14,7 @@ import {
} from "../constants";
import { Cache, CacheLoadOptions, Deferred, Status } from "../types";
import { createDeferred } from "../utils/createDeferred";
import { isPromiseLike } from "../utils/isPromiseLike";
import { mockWeakRef, WeakRefArray } from "../utils/test";
import { useImperativeCacheValue } from "./useImperativeCacheValue";

Expand All @@ -22,21 +24,35 @@ describe("useImperativeCacheValue", () => {
let cache: Cache<[string], Value>;
let fetch: jest.Mock<Promise<Value> | Value, [string, CacheLoadOptions]>;

let container: HTMLDivElement | null = null;
let lastRenderedError: any = undefined;
let lastRenderedStatus: Status | undefined = undefined;
let lastRenderedValue: string | undefined = undefined;

let pendingDeferred: Deferred<Value>[] = [];

function Component({ string }: { string: string }): any {
const result = useImperativeCacheValue(cache, string);
function Component({ cacheKey }: { cacheKey: string }): any {
const result = useImperativeCacheValue(cache, cacheKey);

lastRenderedError = result.error;
lastRenderedStatus = result.status;
lastRenderedValue = result.value;

return null;
}
async function mount() {
container = document.createElement("div");
const root = createRoot(container);
await act(async () => {
root.render(
<>
<ErrorBoundary>
<Component cacheKey="test" />
</ErrorBoundary>
</>
);
});
}

beforeEach(() => {
// @ts-ignore
Expand All @@ -51,6 +67,8 @@ describe("useImperativeCacheValue", () => {
return deferred;
});

container = null;

cache = createCache<[string], Value>({
load: fetch,
});
Expand All @@ -65,11 +83,7 @@ describe("useImperativeCacheValue", () => {
it("should return values that have already been loaded", async () => {
cache.cache({ key: "cached" }, "test");

const container = document.createElement("div");
const root = createRoot(container);
await act(async () => {
root.render(<Component string="test" />);
});
await mount();

expect(lastRenderedError).toBeUndefined();
expect(lastRenderedStatus).toBe(STATUS_RESOLVED);
Expand All @@ -79,11 +93,7 @@ describe("useImperativeCacheValue", () => {
it("should fetch values that have not yet been fetched", async () => {
expect(cache.getStatus("test")).toBe(STATUS_NOT_FOUND);

const container = document.createElement("div");
const root = createRoot(container);
await act(async () => {
root.render(<Component string="test" />);
});
await mount();

expect(pendingDeferred).toHaveLength(1);
expect(lastRenderedStatus).toBe(STATUS_PENDING);
Expand All @@ -98,18 +108,21 @@ describe("useImperativeCacheValue", () => {
it("should handle values that are rejected", async () => {
expect(cache.getStatus("test")).toBe(STATUS_NOT_FOUND);

const container = document.createElement("div");
const root = createRoot(container);
await act(async () => {
root.render(<Component string="test" />);
});
await mount();

expect(pendingDeferred).toHaveLength(1);
expect(lastRenderedStatus).toBe(STATUS_PENDING);

await act(async () => pendingDeferred[0].reject("rejected"));
await act(async () => {
try {
const deferred = pendingDeferred[0];
deferred.reject(new Error("rejected"));

expect(lastRenderedError).toBe("rejected");
await deferred;
} catch (error) {}
});

expect(lastRenderedError?.message).toBe("rejected");
expect(lastRenderedStatus).toBe(STATUS_REJECTED);
expect(lastRenderedValue).toBeUndefined();
});
Expand All @@ -118,11 +131,7 @@ describe("useImperativeCacheValue", () => {
cache.readAsync("test");
expect(pendingDeferred).toHaveLength(1);

const container = document.createElement("div");
const root = createRoot(container);
await act(async () => {
root.render(<Component string="test" />);
});
await mount();

expect(lastRenderedStatus).toBe(STATUS_PENDING);

Expand All @@ -134,18 +143,24 @@ describe("useImperativeCacheValue", () => {
});

it("should wait for values that have already been loaded to be rejected", async () => {
cache.readAsync("test");
const value = cache.readAsync("test");
if (isPromiseLike(value)) {
value.then(
() => {},
() => {}
);
}

expect(pendingDeferred).toHaveLength(1);

const container = document.createElement("div");
const root = createRoot(container);
await act(async () => {
root.render(<Component string="test" />);
});
await mount();

expect(lastRenderedStatus).toBe(STATUS_PENDING);

await act(async () => pendingDeferred[0].reject("rejected"));
await act(async () => {
const deferred = pendingDeferred[0];
deferred.reject("rejected");
});

expect(lastRenderedError).toBe("rejected");
expect(lastRenderedStatus).toBe(STATUS_REJECTED);
Expand All @@ -168,11 +183,7 @@ describe("useImperativeCacheValue", () => {
weakRefArray[0].collect();

// Rendering should trigger a re-fetch
const container = document.createElement("div");
const root = createRoot(container);
await act(async () => {
root.render(<Component string="test" />);
});
await mount();

expect(lastRenderedError).toBeUndefined();
expect(lastRenderedStatus).toBe(STATUS_PENDING);
Expand All @@ -187,3 +198,18 @@ describe("useImperativeCacheValue", () => {
});
});
});

type State = { errorMessage: string | null };
class ErrorBoundary extends Component<PropsWithChildren> {
state: State = { errorMessage: null };
static getDerivedStateFromError(error: any): State {
return { errorMessage: typeof error === "string" ? error : error.message };
}
render() {
if (this.state.errorMessage) {
return this.state.errorMessage;
}

return this.props.children;
}
}
2 changes: 1 addition & 1 deletion packages/suspense/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export type UnsubscribeCallback = () => void;

// Convenience type used by Suspense caches.
// Adds the ability to resolve or reject a pending PromiseLike.
export interface Deferred<Type> extends PromiseLike<Type> {
export interface Deferred<Type> extends Promise<Type> {
reject(error: any): void;
resolve(value?: Type): void;
}
Expand Down
Loading

0 comments on commit b50d819

Please sign in to comment.