fix: The server acking an unset should clean the dirty flag. (#116)

Include some docs on commitChanges.

README.md

@@ -104,6 +104,69 @@ See the [sample](https://github.com/homebound-team/form-state/blob/main/src/Form
 
 # Features
 
+## Incremental changedValue/dirty Management
+
+In general, we have two types of forms:
+
+- `autoSave` forms
+- `submit` forms
+
+### autoSave forms
+
+For auto save forms, the expectation is that you should:
+
+- Pass `useFormState` an `init.input` and `init.map` that updates the form-state from the initial GraphQL request & any cache updates
+- Pass `useFormState` an `autoSave` lambda that calls your GraphQL mutation, using `changedValue`.
+- Have your `save` mutation response return the acked/updated entity/fragment
+
+  ```ts
+  autoSave: async () => {
+    const input = formState.changedValue;
+    await saveAuthor(input);
+  };
+  ```
+
+With these in place, we will correctly handle interleaved edits/saves, i.e.:
+
+1. User changes `{ firstName: bob }`
+2. We submit `{ id: 1, firstName: bob }` to the backend
+3. While waiting for the response, the user sets `{ lastName: smith }`
+4. The GraphQL mutation acks that `{ id: 1, firstName: bob }` is committed
+5. The `init.map` updates `formState` to realize `firstName` is no longer dirty, but `lastName` keeps its WIP change
+6. formState will trigger a 2nd `autoSave` for just the `lastName` change
+
+### Submit forms
+
+For submit forms, the expectation is that you should:
+
+- Pass `useFormState` an `init.input` and `init.map` that updates the form-state from the initial GraphQL request & any cache updates
+- In your `onClick` lambda, use `formState.changedValue` to call your GraphQL mutation
+- Have your `save` mutation response return the acked/updated entity/fragment
+
+If you do this, you should not have to call `commitChanges` manually, because code like:
+
+```ts
+  onClick: async () => {
+    const input = formState.changedValue;
+    await saveAuthor(input);
+    // checks if formState.dirty is true before closing
+    closeModal();
+  };
+```
+
+Will "just work" because the control flow will be:
+
+- User changes `{ firstName: bob }` and clicks Submit
+- `onClick` runs and we submit `{ id: 1, firstName: bob }` to `saveAuthor`
+- _Before_ the `await` promise resolves, the GraphQL response of `saveAuthor { ...AuthorFragment }` will:
+  - Update the apollo cache
+  - Re-render the `AuthorEditor` component with the new data
+  - Call `init.map` to update `formState` with the new data
+  - Realize the `firstName` is no longer dirty
+- When `closeModal` runs, no "You have unsaved changes?" will appear
+
+Basically, in a correctly-setup form, you should never have to call `commitChanges` manually, and doing so risks losing edits that the user made while any saves (either auto save or submit save) were in-flight.
+
 ## Fragments
 
 Normally, form-state expects all fields in the form to be inputs to the GraphQL mutation/wire call. For example, the `author.firstName` field will always be submitted to the `saveAuthor` mutation (albeit with `author.changedValue` you can have `firstName` conditionally included).

src/fields/valueField.ts

@@ -213,7 +213,8 @@ export function newValueFieldState<T, K extends keyof T>(
         throw new Error(`${String(key)} is currently readOnly`);
       }
 
-      if (opts.refreshing && this.dirty && this.value !== value) {
+      const isAckingUnset = this.value === null && (value === null || value === undefined);
+      if (opts.refreshing && this.dirty && this.value !== value && !isAckingUnset) {
         // Ignore incoming values if we have changes (this.dirty) unless our latest change (this.value)
         // matches the incoming value (value), b/c if it does we should accept it and reset originalValue
         // so that we're no longer dirty.

src/formState.test.tsx

@@ -2,7 +2,7 @@ import { autorun, isObservable, makeAutoObservable, observable, reaction } from
 import { ObjectConfig } from "src/config";
 import { f } from "src/configBuilders";
 import { Fragment, ObjectState, createObjectState, fragment } from "src/fields/objectField";
-import { FieldState } from "src/fields/valueField";
+import { FieldState, InternalSetOpts } from "src/fields/valueField";
 import { AuthorAddress, AuthorInput, BookInput, Color, DateOnly, dd100, dd200, jan1, jan2 } from "src/formStateDomain";
 import { required } from "src/rules";
 
@@ -832,6 +832,13 @@ describe("formState", () => {
     expect(a1.firstName.value).toBeNull();
     expect(a1.originalValue.firstName).toBe("asdf");
     expect(a1.firstName.dirty).toBeTruthy();
+    // And when the server acks as null
+    a1.set({ firstName: undefined }, { refreshing: true } as InternalSetOpts);
+    // Then it's no longer dirty
+    expect(a1.firstName.dirty).toBe(false);
+    expect(a1.firstName.originalValue).toBe(undefined);
+    expect(a1.firstName.value).toBe(undefined);
+    expect(a1.changedValue).toEqual({});
   });
 
   it("initializes null values to be undefined", () => {