Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 20 additions & 17 deletions src/AutoSaveStatus/AutoSaveStatusProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,26 +1,27 @@
import React, { PropsWithChildren, useCallback, useEffect, useMemo, useRef, useState } from "react";

export enum AutoSaveStatus {
/** No calls are in-flight or just-recently-saved. */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added descriptions for good measure

IDLE = "idle",
/** A call is actively in-flight. */
SAVING = "saving",
/** A call is no longer actively in-flight, but has recently finished/can show confirmed. */
DONE = "done",
/** A call is no longer actively in-fight, but has errored out. */
ERROR = "error",
}

export interface AutoSaveStatusContextType {
status: AutoSaveStatus;
/** Resets status to IDLE, particularly useful if "Error" or "Done" is stale */
resetStatus: VoidFunction;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed resetStatus with the idea that it's the responsibility of the provider itself to manage when to reset/not-reset status, based on components notifying of their own/individual status.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It already did do that, to an extent.

resetStatus() was a way for components to manually clear our the save status. i.e. if something failed, the user started typing again, and we wanted to show Idle/Ready instead of Error before actually firing a new save.

errors: unknown[];
errors: string[];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the type to string[] b/c one place was passing Errors, another strings, and to be useful to display, the client needs to have some sort of idea of what's here.

/** Notifies AutoSaveContext that a request is in-flight */
triggerAutoSave: VoidFunction;
/** Notifies AutoSaveContext that a request has settled, optionally taking an error */
resolveAutoSave: (error?: unknown) => void;
resolveAutoSave: (error?: string) => void;
}

export const AutoSaveStatusContext = React.createContext<AutoSaveStatusContextType>({
status: AutoSaveStatus.IDLE,
resetStatus() {},
errors: [],
triggerAutoSave() {},
resolveAutoSave() {},
Expand All @@ -31,13 +32,20 @@ type AutoSaveStatusProviderProps = PropsWithChildren<{
resetToIdleTimeout?: number;
}>;

/**
* Provides an app-wide-ish store of in-flight/save status.
*
* Generally there will be only a single `AutoSaveStatusProvider` at the top of the app's
* component tree, although you could also have one inside a modal or drawer component
* to more locally capture/display loading/save status.
*/
export function AutoSaveStatusProvider({ children, resetToIdleTimeout = 6_000 }: AutoSaveStatusProviderProps) {
const [status, setStatus] = useState(AutoSaveStatus.IDLE);
const [errors, setErrors] = useState<unknown[]>([]);
const [errors, setErrors] = useState<string[]>([]);
const [inFlight, setInFlight] = useState(0);
const resetToIdleTimeoutRef = useRef<number | null>(null);

/** Handles setting Status */
// We always derive Status from inFlight/errors
useEffect(() => {
if (inFlight > 0) return setStatus(AutoSaveStatus.SAVING);
if (status === AutoSaveStatus.IDLE) return;
Expand All @@ -50,16 +58,11 @@ export function AutoSaveStatusProvider({ children, resetToIdleTimeout = 6_000 }:
setErrors([]);
}, []);

const resolveAutoSave = useCallback((error?: unknown) => {
const resolveAutoSave = useCallback((error?: string) => {
setInFlight((c) => Math.max(0, c - 1));
if (error) setErrors((errs) => errs.concat(error));
}, []);

const resetStatus = useCallback(() => {
setStatus(AutoSaveStatus.IDLE);
setErrors([]);
}, []);

/** Resets AutoSaveStatus from "Done" to "Idle" after a timeout, if one is provided */
useEffect(() => {
if (resetToIdleTimeout === undefined) return;
Expand All @@ -71,14 +74,14 @@ export function AutoSaveStatusProvider({ children, resetToIdleTimeout = 6_000 }:
if (resetToIdleTimeoutRef.current) clearTimeout(resetToIdleTimeoutRef.current);

resetToIdleTimeoutRef.current = window.setTimeout(() => {
resetStatus();
setStatus(AutoSaveStatus.IDLE);
setErrors([]);
resetToIdleTimeoutRef.current = null;
}, resetToIdleTimeout);
}, [resetStatus, resetToIdleTimeout, status]);
}, [resetToIdleTimeout, status]);

const value = useMemo(() => ({ status, resetStatus, errors, triggerAutoSave, resolveAutoSave }), [
const value = useMemo(() => ({ status, errors, triggerAutoSave, resolveAutoSave }), [
errors,
resetStatus,
resolveAutoSave,
status,
triggerAutoSave,
Expand Down
102 changes: 28 additions & 74 deletions src/AutoSaveStatus/useAutoSaveStatus.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { act, renderHook } from "@testing-library/react-hooks";
import { AutoSaveStatus, AutoSaveStatusProvider } from "./AutoSaveStatusProvider";
import { useContext } from "react";
import { AutoSaveStatus, AutoSaveStatusContext, AutoSaveStatusProvider } from "./AutoSaveStatusProvider";
import { useAutoSaveStatus } from "./useAutoSaveStatus";

const wrapper = AutoSaveStatusProvider;

describe(useAutoSaveStatus, () => {
/** The internal setTimeout running after tests is spamming the console, so cancel them all here */
afterEach(() => {
Expand All @@ -15,69 +18,47 @@ describe(useAutoSaveStatus, () => {
});

it("renders with a provider", () => {
const { result } = renderHook(() => useAutoSaveStatus(), {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of these renderHook changes are just using the wrapper alias.

wrapper: ({ children }) => <AutoSaveStatusProvider>{children}</AutoSaveStatusProvider>,
});
const { result } = renderHook(() => useAutoSaveStatus(), { wrapper });

expect(result.current.status).toBe(AutoSaveStatus.IDLE);
});

it("indicates when something is in-flight", () => {
const { result } = renderHook(() => useAutoSaveStatus(), {
wrapper: ({ children }) => <AutoSaveStatusProvider>{children}</AutoSaveStatusProvider>,
});
const { result } = renderHook(() => useAutoSaveStatus(), { wrapper });

act(() => result.current.triggerAutoSave());
act(() => result.current.setSaving(true));

expect(result.current.status).toBe(AutoSaveStatus.SAVING);
});

it("indicates when a request has settled", () => {
const { result } = renderHook(() => useAutoSaveStatus(), {
wrapper: ({ children }) => <AutoSaveStatusProvider>{children}</AutoSaveStatusProvider>,
});
const { result } = renderHook(() => useAutoSaveStatus(), { wrapper });

act(() => result.current.triggerAutoSave());
act(() => result.current.resolveAutoSave());
act(() => result.current.setSaving(true));
act(() => result.current.setSaving(false));

expect(result.current.status).toBe(AutoSaveStatus.DONE);
});

it("indicates when an error happened", () => {
const { result } = renderHook(() => useAutoSaveStatus(), {
wrapper: ({ children }) => <AutoSaveStatusProvider>{children}</AutoSaveStatusProvider>,
});
const { result } = renderHook(() => useAutoSaveStatus(), { wrapper });

act(() => result.current.triggerAutoSave());
act(() => result.current.resolveAutoSave(new Error("Some error")));
act(() => result.current.setSaving(true));
act(() => result.current.setSaving(false, "Some error"));

expect(result.current.status).toBe(AutoSaveStatus.ERROR);
expect(result.current.errors.length).toBe(1);
});

it("resets status to Idle when told to", () => {
const { result } = renderHook(() => useAutoSaveStatus(), {
wrapper: ({ children }) => <AutoSaveStatusProvider>{children}</AutoSaveStatusProvider>,
});

act(() => result.current.triggerAutoSave());
act(() => result.current.resolveAutoSave());
act(() => result.current.resetStatus());

expect(result.current.status).toBe(AutoSaveStatus.IDLE);
});

it("status goes through the full lifecycle when passed a reset timeout", async () => {
// Given a timeout has been passed to `useAutoSave()`
const { result } = renderHook(() => useAutoSaveStatus(), {
wrapper: ({ children }) => <AutoSaveStatusProvider resetToIdleTimeout={1_000}>{children}</AutoSaveStatusProvider>,
});
const { result } = renderHook(() => useAutoSaveStatus(), { wrapper });
// When we trigger a save
act(() => result.current.triggerAutoSave());
act(() => result.current.setSaving(true));
// Then status is Saving
expect(result.current.status).toBe(AutoSaveStatus.SAVING);
// And when we trigger a resolution
act(() => result.current.resolveAutoSave());
act(() => result.current.setSaving(false));
// Then status is Done
expect(result.current.status).toBe(AutoSaveStatus.DONE);
// But when the timer runs out
Expand All @@ -89,25 +70,21 @@ describe(useAutoSaveStatus, () => {
});

it("clears errors on reset status", () => {
const { result } = renderHook(() => useAutoSaveStatus(), {
wrapper: ({ children }) => <AutoSaveStatusProvider>{children}</AutoSaveStatusProvider>,
});
act(() => result.current.triggerAutoSave());
act(() => result.current.resolveAutoSave(new Error("some error")));
const { result } = renderHook(() => useAutoSaveStatus(), { wrapper });
act(() => result.current.setSaving(true));
act(() => result.current.setSaving(false, "some error"));
expect(result.current.errors.length).toBe(1);
act(() => result.current.resetStatus());
act(() => result.current.setSaving(true));
expect(result.current.errors.length).toBe(0);
expect(result.current.status).toBe(AutoSaveStatus.IDLE);
expect(result.current.status).toBe(AutoSaveStatus.SAVING);
});

it("does not automatically invoke reset timeout if there are errors", () => {
// Given a timeout has been passed to `useAutoSave()`
const { result } = renderHook(() => useAutoSaveStatus(), {
wrapper: ({ children }) => <AutoSaveStatusProvider>{children}</AutoSaveStatusProvider>,
});
const { result } = renderHook(() => useAutoSaveStatus(), { wrapper });

act(() => result.current.triggerAutoSave());
act(() => result.current.resolveAutoSave(new Error("Some error")));
act(() => result.current.setSaving(true));
act(() => result.current.setSaving(false, "Some error"));
act(() => {
jest.runOnlyPendingTimers();
});
Expand All @@ -116,27 +93,8 @@ describe(useAutoSaveStatus, () => {
expect(result.current.errors.length).toBe(1);
});

it("does allow manual resetting even if there are errors", () => {
// Given a timeout has been passed to `useAutoSave()`
const { result } = renderHook(() => useAutoSaveStatus(), {
wrapper: ({ children }) => <AutoSaveStatusProvider>{children}</AutoSaveStatusProvider>,
});

act(() => result.current.triggerAutoSave());
act(() => result.current.resolveAutoSave(new Error("Some error")));
act(() => {
jest.runOnlyPendingTimers();
});
act(() => result.current.resetStatus());

expect(result.current.status).toBe(AutoSaveStatus.IDLE);
expect(result.current.errors.length).toBe(0);
});

it("handles multiple in-flight requests", () => {
const { result } = renderHook(() => useAutoSaveStatus(), {
wrapper: ({ children }) => <AutoSaveStatusProvider>{children}</AutoSaveStatusProvider>,
});
const { result } = renderHook(() => useContext(AutoSaveStatusContext), { wrapper });

// When we trigger 2 AutoSaves and only resolve 1
act(() => result.current.triggerAutoSave());
Expand All @@ -154,21 +112,17 @@ describe(useAutoSaveStatus, () => {
});

it("clears errors when a new save is triggered", () => {
const { result } = renderHook(() => useAutoSaveStatus(), {
wrapper: ({ children }) => <AutoSaveStatusProvider>{children}</AutoSaveStatusProvider>,
});
const { result } = renderHook(() => useContext(AutoSaveStatusContext), { wrapper });

act(() => result.current.triggerAutoSave());
act(() => result.current.resolveAutoSave(new Error("some error")));
act(() => result.current.resolveAutoSave("some error"));
act(() => result.current.triggerAutoSave());

expect(result.current.errors.length).toBe(0);
});

it("handles calling resolve too much", () => {
const { result } = renderHook(() => useAutoSaveStatus(), {
wrapper: ({ children }) => <AutoSaveStatusProvider>{children}</AutoSaveStatusProvider>,
});
const { result } = renderHook(() => useContext(AutoSaveStatusContext), { wrapper });

// When save hasn't been invoked yet
act(() => result.current.resolveAutoSave());
Expand Down
58 changes: 54 additions & 4 deletions src/AutoSaveStatus/useAutoSaveStatus.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,56 @@
import { useContext } from "react";
import { AutoSaveStatusContext } from "./AutoSaveStatusProvider";
import { useCallback, useContext, useEffect, useRef } from "react";
import { AutoSaveStatus, AutoSaveStatusContext } from "./AutoSaveStatusProvider";

export function useAutoSaveStatus() {
return useContext(AutoSaveStatusContext);
export interface AutoSaveStatusHook {
status: AutoSaveStatus;
errors: string[];
/**
* Sets the current component's saving state.
*
* If `error` is passed, it will be added as `errors`; note that we assume `error`
* will be passed on the saving `true` -> `false` transition.
*/
setSaving(saving: boolean, error?: string): void;
}

/**
* Provides the current auto-save `status` as well as a `setSaving` setter
* to easily flag the current component's saving state as true/false.
*
* If your component makes multiple API calls, you can also use two `useAutoSaveStatus`
* hooks, i.e.:
*
* ```
* const { setSaving: setLoadingA } = useAutoSaveStatus();
* const { setSaving: setLoadingB } = useAutoSaveStatus();
* ```
*
* Also ideally your application's infra will automatically integrate `useAutoSaveStatus`
* into all/most wire calls, i.e. by having your own `useMutation` wrapper.
*/
export function useAutoSaveStatus(): AutoSaveStatusHook {
const { status, errors, triggerAutoSave, resolveAutoSave } = useContext(AutoSaveStatusContext);

// Keep a ref to our current value so that we can resolveAutoSave on unmount
const isSaving = useRef(false);

// Make a setter that can be called on every render but only trigger/resolve if saving changed
const setSaving = useCallback(
(saving: boolean, error?: string) => {
if (saving !== isSaving.current) {
saving ? triggerAutoSave() : resolveAutoSave(error);
isSaving.current = saving;
}
},
[triggerAutoSave, resolveAutoSave],
);

// Ensure we resolveAutoSave on unmount
useEffect(() => {
return () => {
isSaving.current && resolveAutoSave();
};
}, [resolveAutoSave]);

return { status, errors, setSaving };
}
13 changes: 7 additions & 6 deletions src/useFormState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ let pendingAutoSave = false;
*/
export function useFormState<T, I>(opts: UseFormStateOpts<T, I>): ObjectState<T> {
const { config, init, addRules, readOnly = false, loading, autoSave } = opts;
const autoSaveStatusContext = useAutoSaveStatus();
// ...should this be deleted b/c we are directly instrumenting Apollo now?
const { setSaving } = useAutoSaveStatus();

// Use a ref so our memo'ized `onBlur` always see the latest value
const autoSaveRef = useRef<((state: ObjectState<T>) => void) | undefined>(autoSave);
Expand Down Expand Up @@ -116,14 +117,14 @@ export function useFormState<T, I>(opts: UseFormStateOpts<T, I>): ObjectState<T>
// user's autoSave function itself wants to call a .set.
const promise = autoSaveRef.current!(form);
isAutoSaving = "in-flight";
autoSaveStatusContext.triggerAutoSave();
setSaving(true);
await promise;
} catch (e) {
maybeError = String(e);
throw e;
} finally {
isAutoSaving = false;
autoSaveStatusContext.resolveAutoSave(maybeError?.toString());
setSaving(false, maybeError?.toString());
if (pendingAutoSave) {
pendingAutoSave = false;
// Push out the follow-up by 1 tick to allow refreshes to happen to potentially
Expand All @@ -138,7 +139,7 @@ export function useFormState<T, I>(opts: UseFormStateOpts<T, I>): ObjectState<T>
const value = firstRunRef.current ? firstInitValue : initValue(config, init);
const form = createObjectState(config, value, { maybeAutoSave });
form.readOnly = readOnly;
setLoading(form, opts);
setLoadingValue(form, opts);
// The identity of `addRules` is not stable, but assume that it is for better UX.
(addRules || (() => {}))(form);
firstRunRef.current = true;
Expand All @@ -159,7 +160,7 @@ export function useFormState<T, I>(opts: UseFormStateOpts<T, I>): ObjectState<T>
firstRunRef.current = false;
return;
}
setLoading(form, opts);
setLoadingValue(form, opts);
(form as any).set(initValue(config, init), { refreshing: true });
}, [form, ...dep]);

Expand All @@ -174,7 +175,7 @@ export function useFormState<T, I>(opts: UseFormStateOpts<T, I>): ObjectState<T>
return form;
}

function setLoading(form: ObjectState<any>, opts: UseFormStateOpts<any, any>) {
function setLoadingValue(form: ObjectState<any>, opts: UseFormStateOpts<any, any>) {
const { loading, init } = opts;
if (loading !== undefined) {
// Prefer the explicit/top-level opts.loading if it's set
Expand Down
Loading