fix: Prevent autoSave fns from calling commitChanges. (#102)

This risks dropping WIP changes, if the user makes a second WIP change
while a first change has its request in-flight (triggered by the autoSave
fn), after the in-flight promise resolves, if the autoSave function then
does a "helpful" commitChanges:

  autoSave(form) {
    await saveMutation(form.changedValue);
    form.commitChanges();
  }

We'll immediately treat all WIP changes as 'accepted', and the 2nd
autoSave, when it goes to run, will see the form as clean/nothing
changed.

Usually this is done on forms that are using `init` instead of the
`init.map/input` pair to get the latest server-side data into the
cache (which is the safer way of "un-dirty-ing" the fields from the
first auto save request.)

src/fields/objectField.ts

@@ -209,6 +209,7 @@ export function newObjectState<T, P = any>(
     _kind: "object",
     _readOnly: false,
     _loading: false,
+    _isAutoSaving: false,
 
     _considerDeleted(): boolean {
       const deleteField = getFields(this).find((f) => f._isDeleteKey);
@@ -304,6 +305,19 @@ export function newObjectState<T, P = any>(
 
     // Saves all current values into _originalValue
     commitChanges() {
+      if (this._isAutoSaving) {
+        // `commitChanges` will mark all WIP changes as committed, which might drop changes if there
+        // is an auto-save in-flight, and the user made more changes, that are waiting for their turn
+        // on the next auto-save request.
+        //
+        // Instead of doing a form-wide "all changes are committed", instead autosave forms should use
+        // init.map/input to have the server's latest results updated within the form, which will then
+        // selectively "un-dirty"/commit the just-saved data, but keep the "still-dirty" WIP data alone.
+        throw new Error(
+          "When using autoSave, you should not manually call commitChanges, instead have init.map/input update the form state",
+        );
+      }
+      // asdf
       getFields(this).forEach((f) => f.commitChanges());
     },
 

src/useFormState.test.tsx

@@ -713,6 +713,37 @@ describe("useFormState", () => {
     expect(r.originalValue).toHaveTextContent("a1");
     expect(r.objectValue).toHaveTextContent("a1");
   });
+
+  it("doesn't allow calling commitChanges", async () => {
+    // Given a component
+    const autoSave = jest.fn();
+    function TestComponent() {
+      type FormValue = AuthorInput;
+      const config: ObjectConfig<FormValue> = authorConfig;
+      const form = useFormState({
+        config,
+        init: { firstName: "f1", lastName: "f1" },
+        autoSave: async (form) => {
+          autoSave(form.changedValue);
+          // And the autoSave functions erroneously calls commitChanges
+          form.commitChanges();
+        },
+      });
+      return <button data-testid="set" onClick={() => form.firstName.set("f2")} />;
+    }
+    const r = await render(<TestComponent />);
+    // When we change a field, and try to commit, it blows up
+    // await expect(clickAndWait(r.set)).rejects.toThrow(
+    //   "When using autoSave, you should not manually call commitChanges",
+    // );
+    //
+    // ...for some reason, I think maybe due to rtl-utils calling runOnlyPendingTimers
+    // instead of runOnlyPendingTimers async, the `clickAndWait` actually resolves
+    // successfully (it's not a rejected promise), and our test technically keeps running,
+    // except then somewhere in the guts of Jest, the fake timer infra realizes that a tick
+    // failed (our autosave is actually fired from a setTimout tick), and so the Jest test
+    // fails anyway, but without anyway for us to catch and do a `.toThrow` assertion.
+  });
 });
 
 const authorConfig: ObjectConfig<AuthorInput> = {

src/useFormState.ts

@@ -114,8 +114,12 @@ export function useFormState<T, I>(opts: UseFormStateOpts<T, I>): ObjectState<T>
           // We use setTimeout as a cheap way to wait until the end of the current event listener
           setTimeout(async () => {
             try {
+              // Tell commitChanges to blow up.
+              (form as any)._isAutoSaving = true;
               // We technically don't flip to in-flight until after the call in case the
-              // user's autoSave function itself wants to call a .set.
+              // user's autoSave function itself wants to call a .set (which would call `maybeAutoSave`,
+              // and we don't want it to see `isAutoSaving=in-flight` and think it "missed the boat",
+              // and so schedule an unnecessary follow-up autosave.)
               const promise = autoSaveRef.current!(form);
               isAutoSaving = "in-flight";
               await promise;
@@ -124,6 +128,7 @@ export function useFormState<T, I>(opts: UseFormStateOpts<T, I>): ObjectState<T>
               throw e;
             } finally {
               isAutoSaving = false;
+              (form as any)._isAutoSaving = false;
               if (pendingAutoSave) {
                 pendingAutoSave = false;
                 // Push out the follow-up by 1 tick to allow refreshes to happen to potentially