Skip to content

Commit

Permalink
feat: Require init.input or init.query. (#104)
Browse files Browse the repository at this point in the history
We've had several auto-saving form-states not realize they should use
init.input + map to get the latest data from the mutation response
automatically included in the form.

In general, having `init: T` is a legacy/rare use case, so let's try
removing it.

Callers that were doing `init: data` can now do `init { input: data }`
although technically the `init.input` will pull in changes to `data` as
its identity changes, while `init: data` would not.

If we roll this out and find a lot of places that need that `init: data`
behavior, we could add something like:

```
  init: {
    input: data,
    initOnlyOnce: true,
  }
```

Or something like that, to be more explicit that they're requesting to
not re-read the `input: data` value on each re-render.
  • Loading branch information
stephenh authored Dec 15, 2023
1 parent 0dd7125 commit 2e97582
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 54 deletions.
12 changes: 7 additions & 5 deletions src/FormStateApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ export function FormStateApp() {
config: formConfig,
// Simulate getting the initial form state back from a server call
init: {
firstName: "a1",
books: [...Array(2)].map((_, i) => ({
title: `b${i}`,
classification: { number: `10${i + 1}`, category: `Test Category ${i}` },
})),
input: {
firstName: "a1",
books: [...Array(2)].map((_, i) => ({
title: `b${i}`,
classification: { number: `10${i + 1}`, category: `Test Category ${i}` },
})),
},
},
addRules(state) {
state.lastName.rules.push(() => {
Expand Down
49 changes: 8 additions & 41 deletions src/useFormState.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,39 +106,6 @@ describe("useFormState", () => {
expect(r.changedValue.textContent).toEqual(JSON.stringify({ firstName: "default" }));
});

it("uses init if set as a value", async () => {
// Given a component
type FormValue = Pick<AuthorInput, "firstName">;
const config: ObjectConfig<FormValue> = { firstName: { type: "value" } };
function TestComponent() {
const [, setTick] = useState(0);
const form = useFormState({
config,
// That's using a raw init value
init: { firstName: "bob" },
});
return (
<div>
<button
data-testid="change"
onClick={() => {
// When that value changes
form.firstName.set("fred");
// And also we re-render the component
setTick(1);
}}
/>
<div data-testid="firstName">{form.firstName.value}</div>
</div>
);
}
const r = await render(<TestComponent />);
expect(r.firstName).toHaveTextContent("bob");
click(r.change);
// Then the change didn't get dropped due to init being unstable
expect(r.firstName).toHaveTextContent("fred");
});

it("doesn't required an init value", async () => {
function TestComponent() {
type FormValue = Pick<AuthorInput, "firstName">;
Expand Down Expand Up @@ -191,7 +158,7 @@ describe("useFormState", () => {
};
// And we start out with data1
const [data, setData] = useState<FormValue>(data1);
const form = useFormState({ config, init: { input: data, map: (d) => d } });
const form = useFormState({ config, init: { input: data } });
function makeLocalChanges() {
form.firstName.value = "local";
form.address.street.value = "local";
Expand Down Expand Up @@ -284,7 +251,7 @@ describe("useFormState", () => {
const [data, setData] = useState<FormValue>(data1);
const form = useFormState({
config,
init: { input: data, map: (d) => d },
init: { input: data },
// And the form is read only
readOnly: true,
});
Expand Down Expand Up @@ -412,7 +379,7 @@ describe("useFormState", () => {
const data2 = { books: [{ title: "Title 1" }] };
const form = useFormState({
config: authorWithBooksConfig,
init: { input: data, map: (d) => d, ifUndefined: { books: [] } },
init: { input: data, ifUndefined: { books: [] } },
autoSave,
});
return (
Expand Down Expand Up @@ -458,7 +425,7 @@ describe("useFormState", () => {
const data = { firstName: "f1", lastName: "f1" };
const form = useFormState({
config,
init: data,
init: { input: data },
// And there is reactive business logic in the `autoSave` method
async autoSave(state) {
state.lastName.set("l2");
Expand Down Expand Up @@ -487,7 +454,7 @@ describe("useFormState", () => {
const data = { firstName: "f1", lastName: "f1" };
const form = useFormState({
config,
init: data,
init: { input: data },
autoSave: (form) => autoSave(form.changedValue),
});
return (
Expand Down Expand Up @@ -602,7 +569,7 @@ describe("useFormState", () => {
);
},
autoSave: (fs) => autoSaveStub(fs.changedValue),
init: { id: "a:1" },
init: { input: { id: "a:1" } },
});
return <TextField field={fs.firstName} />;
}
Expand Down Expand Up @@ -638,7 +605,7 @@ describe("useFormState", () => {
type FormValue = Pick<AuthorInput, "firstName">;
const config: ObjectConfig<FormValue> = { firstName: { type: "value" } };
function TestComponent({ data }: { data: AuthorInput | undefined }) {
const form = useFormState({ config, init: { input: data, map: (d) => d } });
const form = useFormState({ config, init: { input: data } });
return <Observer>{() => <div data-testid="loading">{String(form.loading)}</div>}</Observer>;
}
// And we initially pass in `init.input: undefined`
Expand Down Expand Up @@ -722,7 +689,7 @@ describe("useFormState", () => {
const config: ObjectConfig<FormValue> = authorConfig;
const form = useFormState({
config,
init: { firstName: "f1", lastName: "f1" },
init: { input: { firstName: "f1", lastName: "f1" } },
autoSave: async (form) => {
autoSave(form.changedValue);
// And the autoSave functions erroneously calls commitChanges
Expand Down
10 changes: 8 additions & 2 deletions src/useFormState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export type Query<I> = { data: I; loading: boolean; error?: any };

export type InputAndMap<T, I> = {
input: I;
map: (input: Exclude<I, null | undefined>) => T;
map?: (input: Exclude<I, null | undefined>) => T;
ifUndefined?: T;
};

Expand All @@ -19,6 +19,12 @@ export type QueryAndMap<T, I> = {
ifUndefined?: T;
};

/**
* The opts has for `useFormState`.
*
* @typeparam T the form type, which is usually as close as possible to your *GraphQL input*
* @typeparam I the *form input* type, which is usually the *GraphQL output* type, i.e. the type of the response from your GraphQL query
*/
export type UseFormStateOpts<T, I> = {
/** The form configuration, should be a module-level const or useMemo'd. */
config: ObjectConfig<T>;
Expand All @@ -40,7 +46,7 @@ export type UseFormStateOpts<T, I> = {
* only call `init.map` if it's set, otherwise we'll use `init.ifDefined` or `{}`, saving you
* from having to null check within your `init.map` function.
*/
init?: T | InputAndMap<T, I> | QueryAndMap<T, I>;
init?: InputAndMap<T, I> | QueryAndMap<T, I>;

/**
* A hook to add custom, cross-field validation rules that can be difficult to setup directly in the config DSL.
Expand Down
23 changes: 21 additions & 2 deletions src/useFormStates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ import { initValue } from "src/utils";

export type ObjectStateCache<T, I> = Record<string, [ObjectState<T>, I]>;

/**
* The opts has for `useFormStates`.
*
* @typeparam T the form type, which is usually as close as possible to your *GraphQL input*
* @typeparam I the *form input* type, which is usually the *GraphQL output* type, i.e. the type of the response from your GraphQL query
*/
type UseFormStatesOpts<T, I> = {
/**
* The config to use for each form state.
Expand Down Expand Up @@ -54,6 +60,19 @@ type UseFormStatesHook<T, I> = {
getFormState: (input: I, opts?: { readOnly?: boolean }) => ObjectState<T>;
};

/**
* A hook to manage many "mini-forms" on a single page, typically one form per row
* in a table.
*
* This hook basically provides the page/table with a cache, so each table row naively ask "what's
* the form state for this given row's data?" and get back a new-or-existing `ObjectState` instance
* that, if already existing, still has any of the user's WIP changes.
*
* Each mini-form/row can have its own autoSave calls, independent of the other rows.
*
* @typeparam T the form type, which is usually as close as possible to your *GraphQL input*
* @typeparam I the *form input* type, which is usually the *GraphQL output* type, i.e. the type of the response from your GraphQL query
*/
export function useFormStates<T, I = T>(opts: UseFormStatesOpts<T, I>): UseFormStatesHook<T, I> {
const { config, autoSave, getId, map, addRules, readOnly = false } = opts;

Expand Down Expand Up @@ -109,7 +128,7 @@ export function useFormStates<T, I = T>(opts: UseFormStatesOpts<T, I>): UseFormS

// If it didn't exist, then add to the cache.
if (!form) {
form = createObjectState(config, initValue(config, map ? { map, input } : input), {
form = createObjectState(config, initValue(config, { map, input }), {
maybeAutoSave: () => maybeAutoSave(form),
});
if (addRules) {
Expand All @@ -120,7 +139,7 @@ export function useFormStates<T, I = T>(opts: UseFormStatesOpts<T, I>): UseFormS

// If the source of truth changed, then update the existing state and return it.
if (existing && existing[1] !== input) {
(form as any as ObjectStateInternal<any>).set(initValue(config, map ? { map, input } : input), {
(form as any as ObjectStateInternal<any>).set(initValue(config, { map, input }), {
refreshing: true,
});
existing[1] = input;
Expand Down
11 changes: 7 additions & 4 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,24 @@ export function assertNever(x: never): never {
throw new Error("Unexpected object: " + x);
}

/** Introspects the `init` prop to see if has a `map` function/etc. and returns the form value. */
/** Introspects the `init` prop to see if it has a `map` function/etc. and returns the form value. */
export function initValue<T>(config: ObjectConfig<T>, init: any): T {
let value: any;
if (isInput(init)) {
value = init.input ? init.map(init.input) : init.ifUndefined;
value = init.input ? (init.map ? init.map(init.input) : init.input) : init.ifUndefined;
} else if (isQuery(init)) {
value = init.query.data ? init.map(init.query.data) : init.ifUndefined;
} else if (init === undefined) {
// allow completely undefined init
} else {
value = init;
throw new Error("init must have an input or query key");
}
// Given our form config, pick out only the subset of fields out of `value` (unless it's a mobx class)
return pickFields(config, value ?? {}) as T;
}

export function isInput<T, I>(init: UseFormStateOpts<T, I>["init"]): init is InputAndMap<T, I> {
return !!init && typeof init === "object" && "input" in init && "map" in init;
return !!init && typeof init === "object" && "input" in init;
}

export function isQuery<T, I>(init: UseFormStateOpts<T, I>["init"]): init is QueryAndMap<T, I> {
Expand Down

0 comments on commit 2e97582

Please sign in to comment.