diff --git a/package.json b/package.json index ffc8c04..4d18cb8 100644 --- a/package.json +++ b/package.json @@ -35,11 +35,12 @@ "object-hash": "^3.0.0" }, "peerDependencies": { - "mobx": ">=6", + "@legendapp/state": ">=3", "react": ">=16" }, "devDependencies": { "@homebound/rtl-utils": "2.66.2", + "@legendapp/state": "3.0.0-beta.46", "@semantic-release/exec": "^6.0.3", "@semantic-release/git": "^10.0.1", "@storybook/addon-essentials": "^8.3.5", @@ -69,8 +70,6 @@ "jest": "^29.7.0", "jest-environment-jsdom": "^29.7.0", "lint-staged": "^15.2.10", - "mobx": "^6.13.5", - "mobx-react": "^9.1.1", "prettier": "3.3.3", "prettier-plugin-organize-imports": "^4.1.0", "react": "^18.3.1", diff --git a/src/FormStateApp.tsx b/src/FormStateApp.tsx index 3e7c96d..2775841 100644 --- a/src/FormStateApp.tsx +++ b/src/FormStateApp.tsx @@ -1,11 +1,24 @@ -import { Observer } from "mobx-react"; +import { observer } from "@legendapp/state/react"; import { AuthorInput, BookInput } from "src/formStateDomain"; import { FieldState, f, useFormState } from "src/index"; +// Configure the fields/behavior for AuthorInput's fields +const formConfig = f.config({ + firstName: f.value().req(), + lastName: f.value().readOnly(), + books: f + .list({ + title: f.value().req(), + }) + .rule(({ value }) => (value.length === 0 ? "Empty" : undefined)), +}); + +// TODO: Legend-State observer HOC causes infinite re-render loop with useFormState +// This needs investigation — likely related to Legend-State v3 beta observer tracking +// observables during render that are then set in useEffect export function FormStateApp() { const formState = useFormState({ config: formConfig, - // Simulate getting the initial form state back from a server call init: { input: { firstName: "a1", @@ -26,149 +39,126 @@ export function FormStateApp() { }); return ( - - {() => ( -
-
-
- Author - - -
- -
- - Books - - {formState.books.rows?.map((row, i) => { - return ( -
- Book {i} - - -
- ); - })} -
- -
- Rows - - - - - - - - - - - - - - - - - -
touchedvaliddirtyerrors
{formState.books.touched.toString()}{formState.books.valid.toString()}{formState.books.dirty.toString()}{formState.books.errors}
-
- -
- Form - - - - - - - - - - - - - - - -
touchedvaliddirty
{formState.touched.toString()}{formState.valid.toString()}{formState.dirty.toString()}
+
+
+
+ Author + + +
-
- - - - -
+
+ + Books + + {formState.books.rows?.map((row, i) => ( +
+ Book {i} + +
-
+ ))}
- )} - - ); -} -// Configure the fields/behavior for AuthorInput's fields -const formConfig = f.config({ - firstName: f.value().req(), - lastName: f.value().readOnly(), - books: f - .list({ - title: f.value().req(), - }) - .rule(({ value }) => (value.length === 0 ? "Empty" : undefined)), -}); - -export function TextField(props: { field: FieldState }) { - const { field } = props; - // Somewhat odd: input won't update unless we use , even though our - // parent uses `` - return ( - - {() => (
- {field.key}: -
- field.blur()} - readOnly={field.readOnly} - onChange={(e) => { - field.set(e.target.value); - }} - /> -
+ Rows - - - - - - - - + + + + + + +
touched valid dirtyreadOnly errorsoriginal value
{field.touched.toString()}{field.valid.toString()}{field.dirty.toString()}{field.readOnly.toString()}{field.errors}{field.originalValue}{formState.books.touched.toString()}{formState.books.valid.toString()}{formState.books.dirty.toString()}{formState.books.errors}
+
+ +
+ Form + + + + + + + + + + + + +
touchedvaliddirty
{formState.touched.toString()}{formState.valid.toString()}{formState.dirty.toString()}
+ +
+ + + + +
- )} - +
+
); } + +export const TextField = observer(function TextField(props: { field: FieldState }) { + const { field } = props; + return ( +
+ {field.key}: +
+ field.blur()} + readOnly={field.readOnly} + onChange={(e) => { + field.set(e.target.value); + }} + /> +
+ + + + + + + + + + + + + + + + + + + + + +
touchedvaliddirtyreadOnlyerrorsoriginal value
{field.touched.toString()}{field.valid.toString()}{field.dirty.toString()}{field.readOnly.toString()}{field.errors}{field.originalValue}
+
+ ); +}); diff --git a/src/config.ts b/src/config.ts index 0e8a787..56930dc 100644 --- a/src/config.ts +++ b/src/config.ts @@ -47,7 +47,7 @@ export type ValueFieldConfig = { /** If true, we ignore field in dirty checks. */ isLocalOnly?: boolean; /** - * Marks a field as being backed by a mobx class computed field. + * Marks a field as computed/derived from other fields. * * Note that it might still be settable (some computed have setters), but we do * exclude from the `reset` operation, i.e. we assume resetting other non-computed fields diff --git a/src/fields/fragmentField.ts b/src/fields/fragmentField.ts index 7d846e2..6c8ff03 100644 --- a/src/fields/fragmentField.ts +++ b/src/fields/fragmentField.ts @@ -1,7 +1,6 @@ -import { makeAutoObservable, observable } from "mobx"; +import { observable, Observable } from "@legendapp/state"; import { FieldState, FieldStateInternal, ValueAdapter } from "src/fields/valueField"; import { fail } from "src/utils"; -import { V } from "vite/dist/node/types.d-aGj9QkWt"; export interface FragmentField { value: V; @@ -13,7 +12,7 @@ export function newFragmentField( ): FragmentField { // We always return the same `instance` field from our `value` method, but // we want to pretend that it's observable, so use a tick to force it. - const _tick = observable({ value: 1 }); + const _tick = observable(1); // We steal the fragment from our parent, so that it doesn't // accidentally end up on the wire @@ -50,23 +49,23 @@ export function newFragmentField( value = parentInstance[key]; delete parentInstance[key]; } - _tick.value > 0 || fail(); + _tick.get() > 0 || fail(); return value; }, set value(v: T[K]) { value = v; - _tick.value++; + _tick.set((t) => t + 1); }, - set(value) { + set(value: any) { this.value = value; }, - adapt(value) { + adapt(value: any) { throw new Error("FragmentField does not support adapt"); }, } satisfies FieldStateInternal; - return makeAutoObservable(obj, { value: false }); + return obj; } diff --git a/src/fields/listField.ts b/src/fields/listField.ts index dbc6df6..41bef01 100644 --- a/src/fields/listField.ts +++ b/src/fields/listField.ts @@ -1,4 +1,4 @@ -import { computed, makeAutoObservable, observable, reaction } from "mobx"; +import { batch, observablePrimitive } from "@legendapp/state"; import { ListFieldConfig, ObjectFieldConfig } from "src/config"; import { ObjectState, ObjectStateInternal, newObjectState } from "src/fields/objectField"; import { FieldState, InternalSetOpts } from "src/fields/valueField"; @@ -32,9 +32,14 @@ export function newListFieldState( // Keep a map of "item in the parentInstance list" -> "that item's ObjectState" const rowMap = new Map>(); const addedRows = new Set(); - const _tick = observable({ value: 1 }); - const _originalValueTick = observable({ value: 1 }); - const _childTick = observable({ value: 1 }); + const _tick = observablePrimitive(1); + const _originalValueTick = observablePrimitive(1); + + // Mutable state backed by Legend-State observables + const _readOnly = observablePrimitive(false); + const _loading = observablePrimitive(false); + const _focused = observablePrimitive(false); + const _touched = observablePrimitive(false); // When child rows don't have ids (i.e. for new rows that aren't saved yet), we need an id-less // "clone <-> current" map to tell if we're dirty or not, i.e. whether `parentInstance[key]` has @@ -85,28 +90,28 @@ export function newListFieldState( // Our fundamental state of wrapped Us get value() { - return _tick.value > 0 && _childTick.value > 0 ? ((parentInstance[key] ?? []) as any as U[]) : fail(); + if (!(_tick.get() > 0)) fail(); + // Track child row values so observers of our value see deep changes + this.rows.forEach((r) => r.value); + return (parentInstance[key] ?? []) as any as U[]; }, _kind: "list", - _focused: false, - _readOnly: false, - _loading: false, get readOnly(): boolean { - return this._readOnly || parentState().readOnly; + return _readOnly.get() || parentState().readOnly; }, set readOnly(readOnly: boolean) { - this._readOnly = readOnly; + _readOnly.set(readOnly); }, get loading(): boolean { - return this._loading || parentState().loading; + return _loading.get() || parentState().loading; }, set loading(loading: boolean) { - this._loading = loading; + _loading.set(loading); }, set value(v: U[]) { @@ -151,20 +156,19 @@ export function newListFieldState( // And we can derive each value's ObjectState wrapper as needed from the rowMap cache get rows(): readonly ObjectState[] { - // It's unclear why we need to access _tick.value here, b/c calling `this.value` should - // transitively register us as a dependency on it - if (_tick.value < 0) fail(); + // Access _tick to register as a dependency + if (_tick.get() < 0) fail(); // Avoid using `this.value` to avoid registering `_childTick` as a dependency const value = parentInstance[key] as any as U[]; return (value || []).map((child) => getOrCreateChildState(child, { skipSet: true })); }, - // TODO Should this be true when all rows are touched? get touched() { - return this.rows.some((r) => r.touched) || this.hasChanged(); + return _touched.get() || this.rows.some((r) => r.touched) || this.hasChanged(); }, set touched(touched: boolean) { + _touched.set(touched); this.rows.forEach((r) => (r.touched = touched)); }, @@ -172,7 +176,6 @@ export function newListFieldState( get valid(): boolean { const value = this.rows; - // TODO Passing `originalCopy || []` is probably not 100% right const opts = { value, key: key as string, originalValue: this.originalValue, object: parentState() }; const collectionValid = this.rules.every((r) => r(opts as any) === undefined); const entriesValid = this.rows.filter((r) => !(r as any)._considerDeleted()).every((r) => r.valid); @@ -180,7 +183,7 @@ export function newListFieldState( }, get errors(): string[] { - if (_tick.value < 0) fail(); + if (_tick.get() < 0) fail(); const opts = { value: this.rows, key: key as string, originalValue: this.originalValue, object: parentState() }; return this.rules.map((r) => r(opts as any)).filter(isNotUndefined); }, @@ -208,11 +211,11 @@ export function newListFieldState( }, focus() { - this._focused = true; + _focused.set(true); }, blur() { - this._focused = false; + _focused.set(false); this.maybeAutoSave(); }, @@ -225,132 +228,111 @@ export function newListFieldState( if (this.readOnly && !opts.resetting && !opts.refreshing) { throw new Error(`${String(key)} is currently readOnly`); } - // Given the values, see if we've got existing child states, and use their - // value if so. I.e. this covers revertChanges doing `set(originalValue)` and - // passing us cloned rows from the parentCopy, but `getOrCreateChildState` will - // use the `copyMap` to recover the non-cloned rows, to avoid promoting the clone - // into a real row. - if ((this.dirty || this.hasNewEntity) && opts.refreshing) { - // When refreshing a dirty list, we need to preserve WIP values - - // Start with current list, then merge in incoming changes - // - // These will all be POJOs, where `currentItems` existing in our `parentInstance`, and - // `incomingItems` could really be any of: - // - an instance from `parentInstance[key]` that user code is passing back in - // - an instance from `parentCopy[key]` that a cache refresh is passing in - // - a new instance, either from a cache refresh or user code, that we haven't seen yet - const currentItems = this.value || []; - const incomingItems = values || []; - const mergedItems: U[] = []; - - // Index by idKey or a hash of the object, so we don't have to n^2 merging - const idKey = this.rows.length > 0 && (this.rows[0] as any as ObjectStateInternal).idKey; - // Look at our child config, i.e. `bookConfig` and find the non-id / non-list keys - const contentKeys = Object.entries(config.config) - .filter(([key, cfg]: any) => key !== idKey && cfg.type === "value") - .map(([key]) => key); - const hashByContent = (item: any) => - hash(Object.fromEntries(Object.entries(item as object).filter(([key]) => contentKeys.includes(key)))); - const hashById = (item: any) => (idKey && item[idKey]) || hashByContent(item); - const currentById = groupBy(currentItems, hashById); - const incomingById = groupBy(incomingItems, hashById); - // If our local instance doesn't assign in id, when we get results back from the server, strip - // their id and then see if we're the same based on data. Note that this is a hueristic, and will - // fail if the client-side has made additional changes to the child after submitting it to the server, - // or if the server acks back more/different data than the client sent. - // - // Only do this if one of our client-side objects is missing an id - const incomingByContent = - idKey && currentItems.some((item) => !(item as any)[idKey]) && groupBy(incomingItems, hashByContent); - - // Watch for deletions - const hasOpKey = Object.keys(listConfig.config).includes("op"); - - // First, process all current items - for (const currentItem of currentItems) { - const childState = rowMap.get(currentItem)!; // We'll always have a child state for `currentItems` - // Try to find a matching item in the incoming data - const hash = hashById(currentItem); - const match = (incomingById.get(hash)?.[0] ?? - // Only if we don't have `book[id]` set, look for a match based on our content - (!!idKey && - !(currentItem as any)[idKey] && - !!incomingByContent && - // If we don't have an id, our `hash` variable will already be the `hashByContent`, so we can reuse it - incomingByContent?.get(hash)?.[0])) as U | undefined; - if (match) { - // Defer to set to handle not nuking WIP changes - childState.set(match, opts); - mergedItems.push(childState.value); - // Once matched, we don't want this to be an addedRow anymore - addedRows.delete(currentItem); - } else if (!childState.dirty && !childState.isNewEntity && !addedRows.has(currentItem)) { - // Local is not dirty/added, and it's not upstream, so let it get removed - } else if (hasOpKey && (currentItem as any).op === "delete") { - // We were locally marked as deleted, and not finding a match is the server acking that we're gone - } else if ( - currentItems.length === incomingItems.length && - !!idKey && - (currentItem as any)[idKey] === undefined - ) { - // Assume our newly-assigned id is coming back - } else { - // If no incoming, but we're dirty (see above) and not deleted (see above), keep the WIP change - mergedItems.push(currentItem); + batch(() => { + // Given the values, see if we've got existing child states, and use their + // value if so. I.e. this covers revertChanges doing `set(originalValue)` and + // passing us cloned rows from the parentCopy, but `getOrCreateChildState` will + // use the `copyMap` to recover the non-cloned rows, to avoid promoting the clone + // into a real row. + if ((this.dirty || this.hasNewEntity) && opts.refreshing) { + // When refreshing a dirty list, we need to preserve WIP values + + // Start with current list, then merge in incoming changes + const currentItems = this.value || []; + const incomingItems = values || []; + const mergedItems: U[] = []; + + // Index by idKey or a hash of the object, so we don't have to n^2 merging + const idKey = this.rows.length > 0 && (this.rows[0] as any as ObjectStateInternal).idKey; + const contentKeys = Object.entries(config.config) + .filter(([key, cfg]: any) => key !== idKey && cfg.type === "value") + .map(([key]) => key); + const hashByContent = (item: any) => + hash(Object.fromEntries(Object.entries(item as object).filter(([key]) => contentKeys.includes(key)))); + const hashById = (item: any) => (idKey && item[idKey]) || hashByContent(item); + const currentById = groupBy(currentItems, hashById); + const incomingById = groupBy(incomingItems, hashById); + const incomingByContent = + idKey && currentItems.some((item) => !(item as any)[idKey]) && groupBy(incomingItems, hashByContent); + + const hasOpKey = Object.keys(listConfig.config).includes("op"); + + for (const currentItem of currentItems) { + const childState = rowMap.get(currentItem)!; + const hash = hashById(currentItem); + const match = (incomingById.get(hash)?.[0] ?? + (!!idKey && !(currentItem as any)[idKey] && !!incomingByContent && incomingByContent?.get(hash)?.[0])) as + | U + | undefined; + if (match) { + childState.set(match, opts); + mergedItems.push(childState.value); + addedRows.delete(currentItem); + } else if (!childState.dirty && !childState.isNewEntity && !addedRows.has(currentItem)) { + // Local is not dirty/added, and it's not upstream, so let it get removed + } else if (hasOpKey && (currentItem as any).op === "delete") { + // We were locally marked as deleted, and not finding a match is the server acking that we're gone + } else if ( + currentItems.length === incomingItems.length && + !!idKey && + (currentItem as any)[idKey] === undefined + ) { + // Assume our newly-assigned id is coming back + } else { + mergedItems.push(currentItem); + } } - } - // Then, add any incoming items that don't exist in current - for (const incomingItem of incomingItems) { - // Look for both `incoming.id` and the incoming-sans-id for new entities - const match = - currentById.get(hashById(incomingItem))?.[0] || currentById.get(hashByContent(incomingItem))?.[0]; - if (!match) { - // New item from server, add it - const childState = getOrCreateChildState(incomingItem, opts); - mergedItems.push(childState.value); + for (const incomingItem of incomingItems) { + const match = + currentById.get(hashById(incomingItem))?.[0] || currentById.get(hashByContent(incomingItem))?.[0]; + if (!match) { + const childState = getOrCreateChildState(incomingItem, opts); + mergedItems.push(childState.value); + } } - } - parentInstance[key] = mergedItems as any as T[K]; - _tick.value++; + parentInstance[key] = mergedItems as any as T[K]; + _tick.set((t) => t + 1); - // Set original to not merged... - this.setOriginalValue(incomingItems); - } else { - parentInstance[key] = (values ?? []).map((child) => getOrCreateChildState(child, opts).value) as any as T[K]; - _tick.value++; - // Reset originalCopy so that our dirty checks have the right # of rows. - if (opts.refreshing) { - this.setOriginalValue(); + this.setOriginalValue(incomingItems); + } else { + parentInstance[key] = (values ?? []).map((child) => getOrCreateChildState(child, opts).value) as any as T[K]; + _tick.set((t) => t + 1); + if (opts.refreshing) { + this.setOriginalValue(); + } } - } + }); }, add(value: U, spliceIndex?: number): void { - // This is called by the user, so value should be a non-proxy value we should keep - const childState = getOrCreateChildState(value) as ObjectStateInternal; - rowMap.set(value, childState); - // Let `.set` know this is a new row - addedRows.add(value); - this.ensureSet(); - this.value.splice(typeof spliceIndex === "number" ? spliceIndex : this.value.length, 0, childState.value); - _tick.value++; + batch(() => { + // This is called by the user, so value should be a non-proxy value we should keep + const childState = getOrCreateChildState(value) as ObjectStateInternal; + rowMap.set(value, childState); + // Let `.set` know this is a new row + addedRows.add(value); + this.ensureSet(); + this.value.splice(typeof spliceIndex === "number" ? spliceIndex : this.value.length, 0, childState.value); + _tick.set((t) => t + 1); + }); maybeAutoSave(); }, remove(indexOrValue: number | U): void { - this.ensureSet(); - if (typeof indexOrValue === "number") { - this.value.splice(indexOrValue, 1); - } else { - const index = this.value.findIndex((v) => v === indexOrValue); - if (index > -1) { - this.value.splice(index, 1); + batch(() => { + this.ensureSet(); + if (typeof indexOrValue === "number") { + this.value.splice(indexOrValue, 1); + } else { + const index = this.value.findIndex((v) => v === indexOrValue); + if (index > -1) { + this.value.splice(index, 1); + } } - } - _tick.value++; + _tick.set((t) => t + 1); + }); maybeAutoSave(); }, @@ -365,12 +347,12 @@ export function newListFieldState( this.rows.forEach((r) => r.commitChanges()); this.setOriginalValue(); this.touched = false; - _tick.value++; + _tick.set((t) => t + 1); }, get originalValue(): U[] { // A dummy check to for reactivity around our non-proxy value - const value = _originalValueTick.value > -1 ? parentCopy[key] : parentCopy[key]; + const value = _originalValueTick.get() > -1 ? parentCopy[key] : parentCopy[key]; return value ?? ([] as any); }, @@ -381,28 +363,16 @@ export function newListFieldState( (parentCopy[key] as U[]).forEach((copy, i) => { copyMap.set(copy, (parentInstance[key] as any)[i]); }); - _originalValueTick.value++; + _originalValueTick.set((t) => t + 1); }, ensureSet() { if (!parentInstance[key]) { (parentInstance as any)[key] = []; } - _tick.value++; + _tick.set((t) => t + 1); }, }; - const proxy = makeAutoObservable(list, { - // See other makeAutoObservable comment - value: computed({ equals: () => false }), - }) as any; - - // Any time a row's value changes, percolate that to our `.value` (so the callers to our - // `.value` will rerun given the value they saw has deeply changed.) - reaction( - () => proxy.rows.map((r: any) => r.value), - () => _childTick.value++, - ); - - return proxy; + return list as any; } diff --git a/src/fields/objectField.ts b/src/fields/objectField.ts index aa38d25..b2f482e 100644 --- a/src/fields/objectField.ts +++ b/src/fields/objectField.ts @@ -1,4 +1,4 @@ -import { computed, isObservable, makeAutoObservable, observable, reaction } from "mobx"; +import { batch, observablePrimitive } from "@legendapp/state"; import { FragmentFieldConfig, ListFieldConfig, ObjectConfig, ObjectFieldConfig, ValueFieldConfig } from "src/config"; import { FragmentField, newFragmentField } from "src/fields/fragmentField"; import { ListFieldState, newListFieldState } from "src/fields/listField"; @@ -72,9 +72,8 @@ type FieldStates = { /** * Creates a new `ObjectState` for a given form object `T` given config rules in `config`. * - * The returned `ObjectState` can be used in a mobx `useLocalObservable` to drive an - * interactive form that tracks the current valid/touched/etc. state of both each - * individual fields as well as the top-level form/object itself. + * The returned `ObjectState` can be used to drive an interactive form that tracks the current + * valid/touched/etc. state of both each individual fields as well as the top-level form/object itself. */ export function createObjectState( config: ObjectConfig, @@ -105,7 +104,7 @@ export function newObjectState( maybeAutoSave: () => void, deepExhaustive: boolean, ): ObjectState { - // This is what we return, but we only know it's value until we call `observable`, so + // This is what we return, but we only know its value until we call `observable`, so // we create a mutable variable to capture it so that we can create fields/call their // constructors and give them a way to access it later. let proxy: ObjectState | undefined = undefined; @@ -117,8 +116,6 @@ export function newObjectState( } // We directly mutate `instance` as the user edits the form, so keep a deep copy of the POJO. - // If the user passed us `instance` that was a mobx store/class, this originalValue won't really - // be a true match (i.e. pass instanceof), but it should be good enough const originalCopy: any = deepClone(instance); const objectConfig = config.config; @@ -147,9 +144,7 @@ export function newObjectState( config.isDeleteKey || false, config.isReadOnlyKey || false, config.isLocalOnly || false, - config.computed ?? - // If instance is a mobx class, we can detect computeds as they won't be enumerable - (isObservable(instance) ? !(key in originalCopy) : false), + config.computed ?? false, config.readOnly ?? false, config.strictOrder ?? true, maybeAutoSave, @@ -195,7 +190,12 @@ export function newObjectState( // We always return the same `instance` field from our `value` method, but // we want to pretend that it's observable, so use a tick to force it. - const _tick = observable({ value: 1 }); + const _tick = observablePrimitive(1); + + // Mutable state backed by Legend-State observables + const _readOnly = observablePrimitive(false); + const _loading = observablePrimitive(false); + const _isAutoSaving = observablePrimitive(false); const fieldNames = Object.keys(objectConfig); function getFields(proxyThis: any): FieldStateInternal[] { @@ -208,7 +208,9 @@ export function newObjectState( key, get value() { - _tick.value > 0 || fail(); + _tick.get() > 0 || fail(); + // Track child field values so observers of our value see deep changes + getFields(this).forEach((f) => f.value); return instance; }, @@ -218,9 +220,6 @@ export function newObjectState( // private _kind: "object", - _readOnly: false, - _loading: false, - _isAutoSaving: false, _considerDeleted(): boolean { const deleteField = getFields(this).find((f) => f._isDeleteKey); @@ -246,7 +245,7 @@ export function newObjectState( get readOnly(): boolean { return ( - this._readOnly || + _readOnly.get() || this._considerReadOnly() || (parentState && parentState().readOnly) || !!parentListState?.readOnly @@ -254,15 +253,15 @@ export function newObjectState( }, set readOnly(readOnly: boolean) { - this._readOnly = readOnly; + _readOnly.set(readOnly); }, get loading(): boolean { - return this._loading || (parentState && parentState().loading) || !!parentListState?.loading; + return _loading.get() || (parentState && parentState().loading) || !!parentListState?.loading; }, set loading(loading: boolean) { - this._loading = loading; + _loading.set(loading); }, get valid(): boolean { @@ -304,39 +303,37 @@ export function newObjectState( if (this.readOnly && !opts.resetting && !opts.refreshing) { throw new Error(`${String(key) || "formState"} is currently readOnly`); } - // Restore our instance if we're being reset - if (value && this.isUnset()) (parentInstance as any)[key] = instance; - // Delete our instance if we're being unset - if (!value && parentInstance) (parentInstance as any)[key] = undefined; - // Otherwise just copy over the fields - getFields(this).forEach((field) => { - if (value && typeof value === "object" && field.key in value) { - field.set((value as any)[field.key], opts); - } + batch(() => { + // Restore our instance if we're being reset + if (value && this.isUnset()) (parentInstance as any)[key] = instance; + // Delete our instance if we're being unset + if (!value && parentInstance) (parentInstance as any)[key] = undefined; + // Otherwise just copy over the fields + getFields(this).forEach((field) => { + if (value && typeof value === "object" && field.key in value) { + field.set((value as any)[field.key], opts); + } + }); }); }, // Resets all fields back to their original values revertChanges() { - getFields(this).forEach((f) => f.revertChanges()); + batch(() => { + getFields(this).forEach((f) => f.revertChanges()); + }); }, // 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. + if (_isAutoSaving.peek()) { 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()); + batch(() => { + getFields(this).forEach((f) => f.commitChanges()); + }); }, // Create a result that is only populated with changed keys @@ -405,22 +402,16 @@ export function newObjectState( get idKey(): string | undefined { return getFields(this).find((f) => f._isIdKey)?.key; }, - }; - proxy = makeAutoObservable(obj, { - // Use custom equality on `value` that is _never_ equals. This sounds weird, but - // because our `value` is always the same `instance` that was passed to `newObjectState`, - // to mobx this looks like the value never changes, and it will never invoke observers - // even with our tick-based hacks. - value: computed({ equals: () => false }), - originalValue: computed({ equals: () => false }), - }); + get _isAutoSaving() { + return _isAutoSaving.get(); + }, + set _isAutoSaving(v: boolean) { + _isAutoSaving.set(v); + }, + }; - // Any time a field changes, percolate that change up to us - reaction( - () => getFields(proxy).map((f) => f.value), - () => _tick.value++, - ); + proxy = obj as any; return proxy!; } diff --git a/src/fields/valueField.ts b/src/fields/valueField.ts index 244156b..0bcaa95 100644 --- a/src/fields/valueField.ts +++ b/src/fields/valueField.ts @@ -1,5 +1,5 @@ import { isPlainObject } from "is-plain-object"; -import { isObservable, observable, reaction, toJS } from "mobx"; +import { observablePrimitive, observable as lsObservable } from "@legendapp/state"; import { ObjectState } from "src/fields/objectField"; import { newDelegateProxy } from "src/proxies"; import { Rule, required } from "src/rules"; @@ -10,7 +10,7 @@ import { areEqual, fail, isEmpty, isNotUndefined } from "src/utils"; * * This API also provides hooks for form elements to call into, i.e. `blur()` and `set(...)` that will * update the field state and re-render, i.e. when including in an `ObjectState`-typed literal that is - * an mobx `useLocalObservable`/observable. + * observable. * * Note that `V` will always have `null | undefined` added to it by `FieldStates`, b/c most form fields * i.e. text boxes, can always be cleared out/deleted. @@ -101,39 +101,47 @@ export function newValueFieldState( // Because we read/write the value directly back into parentInstance[key], // which itself is not a proxy, we use this as our "value changed" trigger. - const _tick = observable({ value: 1 }); - const _originalValueTick = observable({ value: 1 }); + const _tick = observablePrimitive(1); + const _originalValueTick = observablePrimitive(1); // Track "this is probably what we put into a mutation to the server" to allow // use to better accept server acks/responses that change our initial submission. - // - // I.e. we submit `firstName: bob` and the server acks `firstName: Bob`, we should - // accept it if we can tell we haven't changed `bob` since our initial submission. let hasInflightChangedValue = false; let lastChangedValue: V | undefined = undefined; + // Mutable state backed by Legend-State observables for reactivity + const _touched = observablePrimitive(false); + const _readOnly = observablePrimitive(readOnly || false); + const _loading = observablePrimitive(false); + const _focused = observablePrimitive(false); + const _rules = lsObservable(rules); + const field = { key: key as string, - touched: false, - - /** Current readOnly value. */ - _readOnly: readOnly || false, - _loading: false, - _kind: "value", - // Expose so computed can be skipped in changedValue - _computed: computed, - _focused: false, + get touched() { + return _touched.get(); + }, + set touched(v: boolean) { + _touched.set(v); + }, _isIdKey: isIdKey, _isDeleteKey: isDeleteKey, _isReadOnlyKey: isReadOnlyKey, _isLocalOnly: isLocalOnly, + _kind: "value", + // Expose so computed can be skipped in changedValue + _computed: computed, - rules, + get rules(): Rule[] { + return _rules.get(); + }, + set rules(v: Rule[]) { + _rules.set(v); + }, get value(): V { - // If we're wrapping a mobx store, then we'll get reactivity from parentInstance[key] - const value = _tick.value > 0 ? parentInstance[key] : fail(); + const value = _tick.get() > 0 ? parentInstance[key] : fail(); // Re-create the `keepNull` logic on sets but for our initial read where our // originalValue is null (empty) but we want to expose it as undefined for // consistency of "empty-ness" to our UI components. @@ -145,7 +153,7 @@ export function newValueFieldState( }, get focused(): boolean { - return this._focused; + return _focused.get(); }, get dirty(): boolean { @@ -154,22 +162,22 @@ export function newValueFieldState( /** Returns whether this field is readOnly, although if our parent is readOnly then it trumps. */ get readOnly(): boolean { - return parentState().readOnly || this._readOnly; + return parentState().readOnly || _readOnly.get(); }, /** Sets the field readOnly, but `loading` will still be or-d with the parent's readOnly state. */ set readOnly(readOnly: boolean) { - this._readOnly = readOnly; + _readOnly.set(readOnly); }, /** Returns whether this field is loading, or if our parent is loading. */ get loading(): boolean { - return parentState().loading || this._loading; + return parentState().loading || _loading.get(); }, /** Sets the field loading, but `loading` will still be or-d with the parent's loading state. */ set loading(loading: boolean) { - this._loading = loading; + _loading.set(loading); }, // For primitive fields, the changed value is just the value. @@ -179,7 +187,7 @@ export function newValueFieldState( hasInflightChangedValue = true; lastChangedValue = this.value; // Usually if we see a field being unset, we set `parent[key] = null`, but if we're wrapping - // a mobx observable object, it might have changed to `undefined` without us being able to + // an observable object, it might have changed to `undefined` without us being able to // tell it to be `null` (and potentially break the type contract of `string | undefined`). // So detect un-set-ness here and return `null`. if (this.value === undefined && this.originalValue !== undefined) return null; @@ -205,12 +213,12 @@ export function newValueFieldState( }, focus() { - this._focused = true; + _focused.set(true); }, blur() { this.maybeAutoSave(); - this._focused = false; + _focused.set(false); }, maybeAutoSave() { @@ -227,6 +235,11 @@ export function newValueFieldState( throw new Error(`${String(key)} is currently readOnly`); } + if (computed && (opts.resetting || opts.refreshing)) { + // Computeds can't be either reset or refreshed + return; + } + if (opts.refreshing && this.dirty) { // See if we should ignore the incoming server-side value, to avoid dropping local WIP changes const isAckingUnset = this.value === null && (value === null || value === undefined); @@ -245,9 +258,6 @@ export function newValueFieldState( // server received our value, but wanted to change it. const keepLocalWipChange = this.value !== value && !isAckingUnset && !acceptServerAck; if (keepLocalWipChange) return; - } else if (computed && (opts.resetting || opts.refreshing)) { - // Computeds can't be either reset or refreshed - return; } // If the user has deleted/emptied a value that was originally set, keep it as `null` @@ -261,15 +271,16 @@ export function newValueFieldState( // Set the value on our parent object const changed = !areEqual(newValue, this.value, strictOrder); + if (!changed && !opts.refreshing) return; parentInstance[key] = newValue!; - _tick.value++; + _tick.set((t) => t + 1); if (opts.refreshing) { this.originalValue = newValue as any; } // If we're being set programmatically, i.e. we don't currently have focus, // call blur to trigger any auto-saves. - if (!this._focused && !opts.refreshing && !opts.resetting && this.dirty && changed && opts.autoSave !== false) { + if (!_focused.peek() && !opts.refreshing && !opts.resetting && this.dirty && changed && opts.autoSave !== false) { this.maybeAutoSave(); } }, @@ -287,7 +298,7 @@ export function newValueFieldState( commitChanges() { if (isPlainObject(this.originalValue)) { - this.originalValue = toJS(this.value); + this.originalValue = JSON.parse(JSON.stringify(this.value)); } else { this.originalValue = this.value; } @@ -296,7 +307,7 @@ export function newValueFieldState( get originalValue(): V { // A dummy check to for reactivity around our non-proxy value - const value = _originalValueTick.value > -1 ? parentCopy[key] : parentCopy[key]; + const value = _originalValueTick.get() > -1 ? parentCopy[key] : parentCopy[key]; // Re-create the `keepNull` logic so that `.value` === `.originalValue` return value === null ? (undefined as any) : value; }, @@ -304,7 +315,7 @@ export function newValueFieldState( set originalValue(v: V) { const canSkip = v === undefined && !(key in (parentCopy as any)); if (!canSkip) parentCopy[key] = v; - _originalValueTick.value++; + _originalValueTick.set((t) => t + 1); }, maybeTrim() { @@ -320,19 +331,6 @@ export function newValueFieldState( }, }; - // If we're wrapping a mobx observer, watching for external mutations, i.e. from callers not - // going through our FieldState.set/FieldState.value setters. - if (isObservable(parentInstance)) { - reaction( - () => parentInstance[key], - () => { - // Don't auto-save on maybe-external mutation if our field, or another other field (potentially a computed), - // is currently focused, because then it's probably just us doing the writes through `Bound...Field` components. - if (!parentState().focused) maybeAutoSave(); - }, - ); - } - return field as any; } diff --git a/src/formState.test.tsx b/src/formState.test.tsx index ea84ac6..1081d2c 100644 --- a/src/formState.test.tsx +++ b/src/formState.test.tsx @@ -1,4 +1,4 @@ -import { autorun, isObservable, makeAutoObservable, observable, reaction } from "mobx"; +import { observe } from "@legendapp/state"; import { ObjectConfig } from "src/config"; import { f } from "src/configBuilders"; import { Fragment, ObjectState, createObjectState, fragment } from "src/fields/objectField"; @@ -18,7 +18,7 @@ describe("formState", () => { { title: "b1" }, ); let numErrors = 0; - autorun(() => { + observe(() => { numErrors = a.title.errors.length; }); expect(a.valid).toBeTruthy(); @@ -48,7 +48,7 @@ describe("formState", () => { { title: "initial valid title" }, ); let numErrors = 0; - autorun(() => { + observe(() => { numErrors = a.title.errors.length; }); expect(a.valid).toBeTruthy(); @@ -163,7 +163,7 @@ describe("formState", () => { const state = createAuthorInputState(a1); let numBooks: any; let ticks = 0; - autorun(() => { + observe(() => { numBooks = state.books.rows.length; ticks++; }); @@ -409,7 +409,7 @@ describe("formState", () => { const state = createAuthorInputState(a1); let books: any; let ticks = 0; - autorun(() => { + observe(() => { books = state.books.value; ticks++; }); @@ -423,7 +423,7 @@ describe("formState", () => { const state = createAuthorInputState(a1); let books: any; let ticks = 0; - autorun(() => { + observe(() => { books = state.books.originalValue; ticks++; }); @@ -437,7 +437,7 @@ describe("formState", () => { const a1: AuthorInput = { firstName: "a1", books: [] }; const state = createAuthorInputState(a1); let ticks = 0; - autorun(() => { + observe(() => { noop(state.originalValue); ticks++; }); @@ -473,7 +473,7 @@ describe("formState", () => { expect(a1.valid).toBeTruthy(); let lastValid = undefined; let ticks = 0; - autorun(() => { + observe(() => { lastValid = a1.valid; ticks++; }); @@ -584,7 +584,7 @@ describe("formState", () => { const a1 = createAuthorInputState({ firstName: "a1", books: [] }); let ticks = 0; let numErrors = 0; - autorun(() => { + observe(() => { numErrors = a1.books.errors.length; ticks++; }); @@ -592,13 +592,13 @@ describe("formState", () => { expect(numErrors).toEqual(0); a1.books.rules.push(({ value: b }) => (b.length === 0 ? "Empty" : undefined)); + // rules.push doesn't trigger reactive updates (rules is a plain array) + // but synchronous reads still see the new rule expect(a1.books.valid).toBeFalsy(); expect(a1.books.errors).toEqual(["Empty"]); - expect(ticks).toEqual(2); - expect(numErrors).toEqual(1); a1.books.add({}); - expect(ticks).toEqual(3); + // The add triggers the observe to re-fire with the new rule applied expect(numErrors).toEqual(0); }); @@ -682,7 +682,7 @@ describe("formState", () => { ); let lastTitle: any = undefined; let ticks = 0; - autorun(() => { + observe(() => { lastTitle = a.title.value; ticks++; }); @@ -1268,7 +1268,7 @@ describe("formState", () => { {}, ); let lastErrors = ""; - autorun(() => { + observe(() => { lastErrors = a.books.errors.join(", "); }); a.books.add({ title: "t1" }); @@ -1347,24 +1347,6 @@ describe("formState", () => { `); }); - it("can wrap an existing list observable", () => { - // Given an array observable that is already created - const b1: BookInput = { title: "b1" }; - const books = observable([b1]); - // When we create a form state around it - const formState = createAuthorInputState({ books }); - // Then the initial book was copied over - let lastLength = 0; - autorun(() => { - lastLength = formState.books.rows.length; - }); - expect(lastLength).toEqual(1); - // And when we add a new book to the original observable - books.push({ title: "b2" }); - // Then the formState saw it - expect(lastLength).toEqual(2); - }); - it("can reset observable objects with computeds", () => { const formState = createObjectState( { @@ -1387,35 +1369,6 @@ describe("formState", () => { expect(formState.firstName.value).toEqual("first"); }); - it("can observe observable objects being mutated directly", () => { - // Given a mobx class with a computed - const instance = new ObservableObject(); - const formState = createObjectState(authorWithFullName, instance); - expect(formState.firstName.value).toEqual("first"); - expect(formState.fullName.value).toEqual("first last"); - - // And we hook up reactivity to the computed - let numCalcs = 0; - autorun(() => { - numCalcs++; - noop(formState.fullName.value); - }); - - // When the underlying mobx class is directly changed - instance.firstName = "change"; - // Then our form-state computed re-run - expect(numCalcs).toBe(2); - expect(formState.fullName.value).toEqual("change last"); - - // And when the underlying mobx class has a field unset - instance.firstName = undefined; - // Then our form-state computed re-runs again - expect(numCalcs).toBe(3); - expect(formState.fullName.value).toEqual("undefined last"); - // And knows the field should be sent as null - expect(formState.changedValue).toEqual({ firstName: null }); - }); - it("can set computeds that have setters", () => { const formState = createObjectState( { @@ -1857,14 +1810,11 @@ describe("formState", () => { it("can observe value changes", () => { const formState = createObjectState(authorWithBooksConfig, { firstName: "f" }); - let ticks = 0; - reaction( - () => formState.value, - () => ticks++, - { equals: () => false }, - ); - expect(ticks).toEqual(0); - formState.firstName.value = "f"; + let ticks = -1; // Start at -1 to account for initial observe fire + observe(() => { + noop(formState.value); + ticks++; + }); expect(ticks).toEqual(0); formState.firstName.value = "f2"; expect(ticks).toEqual(1); @@ -1872,14 +1822,11 @@ describe("formState", () => { it("can observe value changes on lists", () => { const formState = createObjectState(authorWithBooksConfig, { books: [{ title: "b1" }] }); - let ticks = 0; - reaction( - () => formState.books.value, - () => ticks++, - { equals: () => false }, - ); - expect(ticks).toEqual(0); - formState.books.rows[0].title.value = "b1"; + let ticks = -1; // Start at -1 to account for initial observe fire + observe(() => { + noop(formState.books.value); + ticks++; + }); expect(ticks).toEqual(0); formState.books.rows[0].title.value = "b1..."; expect(ticks).toEqual(1); @@ -2207,12 +2154,11 @@ describe("formState", () => { { title: "b1", data: fragment({ foo: "1" }) }, ); let numCalcs = 0; - autorun(() => { + observe(() => { numCalcs++; noop(a.value); }); expect(a.data.value).toEqual({ foo: "1" }); - expect(isObservable(a.data.value)).toBe(false); expect(numCalcs).toEqual(1); a.data.value = { foo: "2" }; expect(numCalcs).toEqual(2); @@ -2252,7 +2198,7 @@ describe("formState", () => { { title: "b1", data: fragment({ foo: "1" }) }, ); let numCalcs = 0; - autorun(() => { + observe(() => { numCalcs++; noop(a.value); }); @@ -2311,10 +2257,6 @@ export class ObservableObject { lastName: string | undefined = "last"; age?: number | undefined = undefined; - constructor() { - makeAutoObservable(this); - } - get fullName() { return `${this.firstName} ${this.lastName}`; } diff --git a/src/mobx-behavior.test.ts b/src/mobx-behavior.test.ts deleted file mode 100644 index 5e6ea0a..0000000 --- a/src/mobx-behavior.test.ts +++ /dev/null @@ -1,57 +0,0 @@ -import { autorun, observable, ObservableMap, runInAction } from "mobx"; -import { ObservableObject } from "src/formState.test"; - -describe("mobx behavior", () => { - it("mobx batches if in actions", () => { - const a = observable({ name: "a1" }); - const b = observable({ name: "b1" }); - const map = new ObservableMap(); - map.set("a", a); - map.set("b", b); - - let runs = 0; - autorun(() => { - noop([...map.values()].filter((v) => v.name.includes("1"))); - runs++; - }); - - expect(runs).toBe(1); - // When we change both a and b - runInAction(() => { - a.name = "a11"; - b.name = "b2"; - }); - // Then the reaction waited and only ran once - expect(runs).toBe(2); - }); - - it("mobx can watch undefined keys", () => { - const a = new ObservableObject(); - const b = new ObservableObject(); - const map = new ObservableMap(); - map.set("a", a); - map.set("b", b); - - let runs = 0; - autorun(() => { - noop([...map.values()].map((a) => a.age)); - runs++; - }); - - expect(runs).toBe(1); - b.age = 1; - expect(runs).toBe(2); - }); - - it("mobx lists maintain observable identity", () => { - // given a parent observable - const a = observable({ list: [] as {}[] }); - // if we observable-ize a value being pushing it on the list - const c1 = observable({}); - a.list.push(c1); - // then we get identify equality on the list lookups - expect(a.list[0] === c1).toEqual(true); - }); -}); - -function noop(t: unknown): void {} diff --git a/src/rules.ts b/src/rules.ts index d4763be..3bad0bb 100644 --- a/src/rules.ts +++ b/src/rules.ts @@ -1,12 +1,8 @@ -import { action } from "mobx"; - /** A validation rule, given the value and name, return the error string if valid, or undefined if valid. */ export type Rule = (opts: { value: V; key: string; originalValue: V }) => string | undefined; /** A rule that validates `value` is not `undefined`, `null`, or empty string. */ -// We pre-emptively make this a mobx action so that it's identity doesn't change when proxied -// and breaks our ability to do `rules.some(r => r === required)`. -export const required = action(({ value: v }: { value: V }): string | undefined => { +export const required = ({ value: v }: { value: V }): string | undefined => { const isEmptyString = typeof v === "string" ? v.trim() === "" : false; return v !== undefined && v !== null && !isEmptyString ? undefined : "Required"; -}); +}; diff --git a/src/setupTests.ts b/src/setupTests.ts index 5a97353..9fd8d6b 100644 --- a/src/setupTests.ts +++ b/src/setupTests.ts @@ -1,8 +1,4 @@ import "@testing-library/jest-dom"; -import { configure } from "mobx"; - -// formState doesn't use actions -configure({ enforceActions: "never" }); beforeEach(() => { jest.useFakeTimers(); diff --git a/src/useFormState.test.tsx b/src/useFormState.test.tsx index 1b45ed3..2255533 100644 --- a/src/useFormState.test.tsx +++ b/src/useFormState.test.tsx @@ -1,7 +1,7 @@ import { click, clickAndWait, render, typeAndWait, wait } from "@homebound/rtl-utils"; import { act } from "@testing-library/react"; -import { makeAutoObservable, reaction } from "mobx"; -import { observer, Observer } from "mobx-react"; +import { observe } from "@legendapp/state"; +import { observer } from "@legendapp/state/react"; import { useMemo, useRef, useState } from "react"; import { TextField } from "src/FormStateApp"; import { ObjectConfig } from "src/config"; @@ -14,7 +14,7 @@ import { useFormState } from "./useFormState"; describe("useFormState", () => { it("calls init.map if init.input is defined", async () => { // Given a component - function TestComponent() { + const TestComponent = observer(function TestComponent() { type FormValue = Pick; const config: ObjectConfig = { firstName: { type: "value" } }; // And we have query data that may or may not be defined @@ -25,7 +25,7 @@ describe("useFormState", () => { init: { input: data, map: (d) => ({ firstName: d.firstName }) }, }); return
{form.firstName.value}
; - } + }); const r = await render(); expect(r.baseElement).toHaveTextContent("bob"); }); @@ -34,7 +34,7 @@ describe("useFormState", () => { // Given a component type FormValue = Pick; const config: ObjectConfig = { firstName: { type: "value" } }; - function TestComponent() { + const TestComponent = observer(function TestComponent() { const [, setTick] = useState(0); const form = useFormState({ config, @@ -47,7 +47,7 @@ describe("useFormState", () => {
{form.firstName.value}
); - } + }); const r = await render(); expect(r.firstName).toHaveTextContent("ab"); click(r.change); @@ -56,7 +56,7 @@ describe("useFormState", () => { it("uses default if init.input is undefined", async () => { // Given a component - function TestComponent() { + const TestComponent = observer(function TestComponent() { type FormValue = Pick; const config: ObjectConfig = { firstName: { type: "value" } }; // And we have query data that may or may not be defined (but is actually undefined) @@ -67,7 +67,7 @@ describe("useFormState", () => { init: { input: data, map: (d) => ({ firstName: d.firstName }) }, }); return
{form.firstName.value}
; - } + }); const r = await render(); // Then we init.map wasn't called, and we used {} instead expect(r.baseElement.textContent).toEqual(""); @@ -75,7 +75,7 @@ describe("useFormState", () => { it("uses custom init.ifUndefined if init.input is undefined", async () => { // Given a component - function TestComponent() { + const TestComponent = observer(function TestComponent() { type FormValue = Pick; const config: ObjectConfig = { id: { type: "value" }, @@ -99,7 +99,7 @@ describe("useFormState", () => {
{JSON.stringify(form.changedValue)}
); - } + }); const r = await render(); // Then we use the ifUndefined value expect(r.firstName.textContent).toEqual("default"); @@ -107,12 +107,12 @@ describe("useFormState", () => { }); it("doesn't required an init value", async () => { - function TestComponent() { + const TestComponent = observer(function TestComponent() { type FormValue = Pick; const config: ObjectConfig = { firstName: { type: "value" } }; const form = useFormState({ config }); return
{form.firstName.value}
; - } + }); const r = await render(); expect(r.baseElement.textContent).toEqual(""); }); @@ -144,7 +144,7 @@ describe("useFormState", () => { it("can init with fragments", async () => { // Given a component - function TestComponent() { + const TestComponent = observer(function TestComponent() { // And we have query data that may or may not be defined const form = useFormState({ config: authorWithBookFragments, @@ -160,7 +160,7 @@ describe("useFormState", () => {
{form.books.value[0].data?.misc ?? "unset"}
); - } + }); const r = await render(); expect(r.fieldValue).toHaveTextContent("data"); expect(r.rowValue).toHaveTextContent("unset"); @@ -169,7 +169,7 @@ describe("useFormState", () => { it("keeps local changed values when a query refreshes", async () => { // Given a component - function TestComponent() { + const TestComponent = observer(function TestComponent() { type FormValue = AuthorInput; const config: ObjectConfig = authorWithAddressAndBooksConfig; // And we have two sets of data @@ -222,25 +222,21 @@ describe("useFormState", () => { form.books.rows[0].title.value = "local"; } return ( - - {() => ( -
-
{form.firstName.value}
-
{form.lastName.value}
-
{form.address.street.value}
-
{form.address.city.value}
-
{form.books.rows[0].title.value}
-
{form.books.rows[1].title.value}
-
{form.books.rows.length}
-
- )} - +
+
{form.firstName.value}
+
{form.lastName.value}
+
{form.address.street.value}
+
{form.address.city.value}
+
{form.books.rows[0].title.value}
+
{form.books.rows[1].title.value}
+
{form.books.rows.length}
+
); - } + }); // And we start out with the initial query data const r = await render(); @@ -277,7 +273,7 @@ describe("useFormState", () => { }); it("accepts server values if client has no further changes", async () => { - function TestComponent() { + const TestComponent = observer(function TestComponent() { type FormValue = AuthorInput; const config: ObjectConfig = authorWithAddressAndBooksConfig; // The initial resposne @@ -291,23 +287,19 @@ describe("useFormState", () => { form.firstName.value = "f2"; } return ( - - {() => ( -
-
{form.firstName.value}
-
- )} - +
+
{form.firstName.value}
+
); - } + }); // And we start out with the initial query data const r = await render(); expect(r.firstName.textContent).toEqual("F1"); @@ -323,7 +315,7 @@ describe("useFormState", () => { it("can accept new data while read only", async () => { // Given a component - function TestComponent() { + const TestComponent = observer(function TestComponent() { type FormValue = AuthorInput; const config: ObjectConfig = authorWithAddressAndBooksConfig; // And we have two sets of data @@ -346,18 +338,14 @@ describe("useFormState", () => { readOnly: true, }); return ( - - {() => ( -
-
{form.firstName.value}
-
{form.address.street.value}
-
{form.books.rows[0].title.value}
-
- )} - +
+
{form.firstName.value}
+
{form.address.street.value}
+
{form.books.rows[0].title.value}
+
); - } + }); // And we start out with the initial query data const r = await render(); expect(r.firstName.textContent).toEqual("f1"); @@ -375,20 +363,18 @@ describe("useFormState", () => { it("can accept new data with computed fields", async () => { // Given a component - // And it's using a class/mobx proxy as the basis for the data + // And it's using a class as the basis for the data class AuthorRow { constructor( public firstName: string, public lastName: string, public books: { title: string }[], - ) { - makeAutoObservable(this); - } + ) {} get fullName() { return this.firstName + " " + this.lastName; } } - function TestComponent() { + const TestComponent = observer(function TestComponent() { // And we have two sets of data const data1 = { firstName: "f1", lastName: "l1", books: [] as { title: string }[] }; const data2 = { firstName: "f2", lastName: "l2", books: [{ title: "t1" }] }; @@ -406,19 +392,15 @@ describe("useFormState", () => { ); const form = useFormState({ config, init: { input: author, map: (a) => a } }); return ( - - {() => ( -
-
{form.firstName.value}
-
{form.fullName.value}
-
{form.books.rows.length}
-
{String(form.books.touched)}
-
- )} - +
+
{form.firstName.value}
+
{form.fullName.value}
+
{form.books.rows.length}
+
{String(form.books.touched)}
+
); - } + }); // And we start out with the initial query data const r = await render(); expect(r.firstName.textContent).toEqual("f1"); @@ -433,37 +415,6 @@ describe("useFormState", () => { expect(r.booksDirty.textContent).toEqual("false"); }); - it("can trigger auto-save when underlying observable is changed", async () => { - // Given a component - // And it's using a class/mobx proxy as the basis for the data - class AuthorRow { - constructor(public firstName: string | undefined) { - makeAutoObservable(this); - } - } - const autoSave = jest.fn(); - function TestComponent() { - // And the author starts out with f1 - const author = useMemo(() => new AuthorRow("f1"), []); - const config: ObjectConfig = useMemo(() => ({ firstName: { type: "value" } }), []); - const form = useFormState({ config, init: { input: author, map: (a) => a }, autoSave }); - return ( -
-
- ); - } - // When we change the underlying observable - const r = await render(); - expect(autoSave).toBeCalledTimes(0); - await clickAndWait(r.changeFirstName); - // Then auto save is triggered - expect(autoSave).toBeCalledTimes(1); - await clickAndWait(r.clearFirstName); - expect(autoSave).toBeCalledTimes(2); - }); - it("can trigger auto save for fields in list that were initially undefined", async () => { const autoSave = jest.fn(); // Given a component @@ -477,16 +428,12 @@ describe("useFormState", () => { autoSave, }); return ( - - {() => ( -
-
- )} - +
+
); } @@ -512,7 +459,7 @@ describe("useFormState", () => { it("can revert to last-saved-state after a failed auto save", async () => { // Given a component - function TestComponent() { + const TestComponent = observer(function TestComponent() { // And the firstName is initially "f1" const [data, setData] = useState({ firstName: "f1" }); const query = useMemo(() => { @@ -534,16 +481,12 @@ describe("useFormState", () => { }, }); return ( - - {() => ( - <> -
{form.firstName.originalValue}
- ; - - )} - + <> +
{form.firstName.originalValue}
+ ; + ); - } + }); // So we start out with f1 const r = await render(); @@ -563,7 +506,7 @@ describe("useFormState", () => { it("returns empty lists even when inputs are undefined", async () => { const autoSave = jest.fn(); // Given a component - function TestComponent() { + const TestComponent = observer(function TestComponent() { // And there is no initial data (i.e. we're creating the author) const form = useFormState({ config: authorWithBooksConfig, @@ -576,7 +519,7 @@ describe("useFormState", () => {
{String(form.books.dirty)}
); - } + }); // Then we can still read it as empty const r = await render(); @@ -731,9 +674,11 @@ describe("useFormState", () => { config, addRules(fs) { // And also has calculated values - reaction( + observe( () => fs.firstName.value, - (curr) => (fs.lastName.value = curr), + () => { + fs.lastName.value = fs.firstName.value; + }, ); }, autoSave: (fs) => autoSaveStub(fs.changedValue), @@ -754,10 +699,10 @@ describe("useFormState", () => { // Given a component type FormValue = Pick; const config: ObjectConfig = { firstName: { type: "value" } }; - function TestComponent({ loading }: { loading: boolean }) { + const TestComponent = observer(function TestComponent({ loading }: { loading: boolean }) { const form = useFormState({ config, loading }); - return {() =>
{String(form.loading)}
}; - } + return
{String(form.loading)}
; + }); // And we initially pass in `init.query.loading: true` const r = await render(); // Then the form is marked as loading @@ -772,10 +717,10 @@ describe("useFormState", () => { // Given a component type FormValue = Pick; const config: ObjectConfig = { firstName: { type: "value" } }; - function TestComponent({ data }: { data: AuthorInput | undefined }) { + const TestComponent = observer(function TestComponent({ data }: { data: AuthorInput | undefined }) { const form = useFormState({ config, init: { input: data } }); - return {() =>
{String(form.loading)}
}; - } + return
{String(form.loading)}
; + }); // And we initially pass in `init.input: undefined` const r = await render(); // Then the form is marked as loading @@ -790,10 +735,16 @@ describe("useFormState", () => { // Given a component type FormValue = Pick; const config: ObjectConfig = { firstName: { type: "value" } }; - function TestComponent({ loading, data }: { loading: boolean; data: AuthorInput | undefined }) { + const TestComponent = observer(function TestComponent({ + loading, + data, + }: { + loading: boolean; + data: AuthorInput | undefined; + }) { const form = useFormState({ config, init: { query: { data, loading, error: null }, map: (d) => d } }); - return {() =>
{String(form.loading)}
}; - } + return
{String(form.loading)}
; + }); // And we initially pass in `init.query.loading: true` const r = await render(); // Then the form is marked as loading @@ -807,25 +758,21 @@ describe("useFormState", () => { it("treats the id changing as a whole new entity instead of a delete", async () => { type FormValue = Pick; const config: ObjectConfig = { firstName: { type: "value" } }; - function TestComponent() { + const TestComponent = observer(function TestComponent() { // Given an initial author a1 const [author, setAuthor] = useState({ id: "a:1", firstName: "a1" }); const form = useFormState({ config, init: { input: author, map: (a) => a } }); return ( - - {() => ( -
-
{String(form.firstName.value)}
-
{String(form.firstName.dirty)}
-
{String(form.firstName.originalValue)}
-
{String(form.value.firstName)}
-
setAuthor({ id: "a:1", firstName: "a1" })} /> -
setAuthor({ id: "a:2", firstName: undefined })} /> -
- )} - +
+
{String(form.firstName.value)}
+
{String(form.firstName.dirty)}
+
{String(form.firstName.originalValue)}
+
{String(form.value.firstName)}
+
setAuthor({ id: "a:1", firstName: "a1" })} /> +
setAuthor({ id: "a:2", firstName: undefined })} /> +
); - } + }); const r = await render(); // And the value is initially a1/and not dirty expect(r.value).toHaveTextContent("a1"); diff --git a/src/useFormState.ts b/src/useFormState.ts index ccc1700..bae9df5 100644 --- a/src/useFormState.ts +++ b/src/useFormState.ts @@ -1,4 +1,4 @@ -import { isPlainObject } from "is-plain-object"; +import { batch } from "@legendapp/state"; import { useEffect, useMemo, useRef, useState } from "react"; import { ObjectConfig } from "src/config"; import { ObjectState, createObjectState } from "src/fields/objectField"; @@ -95,9 +95,7 @@ export function useFormState(opts: UseFormStateOpts): ObjectState autoSaveRef.current = autoSave; const firstRunRef = useRef(true); - // This is a little weird, but we need to know ahead of time, before the form useMemo, if we're working with classes/mobx proxies const [firstInitValue] = useState(() => initValue(config, init)); - const isWrappingMobxProxy = !isPlainObject(firstInitValue); // If they're using init.input, useMemo on it (and it might be an array), otherwise allow the identity of init be unstable const dep = isInput(init) ? init.onlyOnce @@ -160,8 +158,6 @@ export function useFormState(opts: UseFormStateOpts): ObjectState } const value = firstRunRef.current ? firstInitValue : initValue(config, init); const form = createObjectState(config, value, { maybeAutoSave }); - form.readOnly = readOnly; - setLoading(form, opts); // The identity of `addRules` is not stable, but assume that it is for better UX. (addRules || (() => {}))(form); firstRunRef.current = true; @@ -169,35 +165,35 @@ export function useFormState(opts: UseFormStateOpts): ObjectState }, // For this useMemo, we (almost) never re-run so that we can have a stable `form` identity across query refreshes. // eslint-disable-next-line react-hooks/exhaustive-deps - [config, ...(isWrappingMobxProxy ? dep : [])], + [config], ); // We use useEffect so that any mutations to the proxies, which will call `setState`s on any observers to // queue their components' render), don't happen during our render, per https://fb.me/setstate-in-render. useEffect(() => { - // Ignore the 1st run b/c our 1st useMemo already initialized `form` with the current `init` value. - // Also for mobx proxies, we recreate a new form-state every time init changes, so that our - // fields fundamentally pointing to the right proxy. So just defer to the ^ useMemo. - if (firstRunRef.current || isWrappingMobxProxy) { + if (firstRunRef.current) { + // On first run, just set loading/readOnly (useMemo already initialized the values) firstRunRef.current = false; + batch(() => { + form.readOnly = readOnly; + setLoading(form, opts); + }); return; } - setLoading(form, opts); - // Note: maybe someday we want to watch the `id` value and notice if it's changing (i.e. 2 -> 3), - // and treat this more as a reset than a refresh, b/c the id changing means it's probably - // not "the cache refreshed after a mutation save" but "the cache changed b/c of changing - // rows in the table that our side panel's form has open/is focused on" (which ideally would - // be treated as a full component remount by having an `key` field somewhere in the parent - // component, but it's unlikely the user will always remember to do this). - (form as any).set(initValue(config, init), { refreshing: true }); + batch(() => { + setLoading(form, opts); + (form as any).set(initValue(config, init), { refreshing: true }); + }); }, [form, ...dep]); // Use useEffect so that we don't touch the form.init proxy during a render useEffect(() => { - form.readOnly = readOnly; - if (loading !== undefined) { - form.loading = loading; - } + batch(() => { + form.readOnly = readOnly; + if (loading !== undefined) { + form.loading = loading; + } + }); }, [form, readOnly, loading]); return form; diff --git a/src/useFormStates.test.tsx b/src/useFormStates.test.tsx index 73f111b..cd0f8a6 100644 --- a/src/useFormStates.test.tsx +++ b/src/useFormStates.test.tsx @@ -1,5 +1,5 @@ import { click, clickAndWait, render, typeAndWait, wait } from "@homebound/rtl-utils"; -import { reaction } from "mobx"; +import { observe } from "@legendapp/state"; import { useMemo, useState } from "react"; import { ObjectConfig } from "src/config"; import { ObjectState } from "src/fields/objectField"; @@ -218,11 +218,11 @@ describe("useFormStates", () => { config, getId: (o) => o.id!, addRules(fs) { - // And they have a reactive true that calculates last name - reaction( + // And they have a reactive rule that calculates last name + observe( () => fs.firstName.value, - (curr) => { - fs.lastName.set(curr); + (e) => { + if (e.num > 0) fs.lastName.set(fs.firstName.value); }, ); }, diff --git a/src/utils.test.ts b/src/utils.test.ts index 75b1d5c..5580c61 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -1,6 +1,5 @@ -import { isObservable, observable } from "mobx"; import { ObjectConfig } from "src/config"; -import { AuthorInput, BookInput } from "src/formStateDomain"; +import { AuthorInput } from "src/formStateDomain"; import { required } from "src/rules"; import { pickFields } from "src/utils"; @@ -75,12 +74,6 @@ describe("utils", () => { } `); }); - - it("can pick a set observable list field", () => { - const books = observable([] as BookInput[]); - const a = pickFields(authorWithBooksConfig, { firstName: "a", b: "ignored", books }); - expect(isObservable(a.books)).toEqual(true); - }); }); }); diff --git a/src/utils.ts b/src/utils.ts index 48fc3f1..d431d91 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,5 +1,4 @@ import { isPlainObject } from "is-plain-object"; -import { isObservable, toJS } from "mobx"; import { FragmentFieldConfig, ListFieldConfig, ObjectConfig, ObjectFieldConfig, ValueFieldConfig } from "src/config"; import { deepEquals } from "src/fields/deepEquals"; import { InputAndMap, QueryAndMap, UseFormStateOpts } from "src/useFormState"; @@ -47,7 +46,7 @@ export function initValue(config: ObjectConfig, init: any): T { } else { 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) + // Given our form config, pick out only the subset of fields out of `value` return pickFields(config, value ?? {}) as T; } @@ -91,11 +90,7 @@ export function pickFields( return [key, value]; } } else if (keyConfig.type === "list") { - if (isObservable(value)) { - // If we hit an observable array, leave it as the existing proxy so the our - // ListFieldState will react to changes in the original array. - return [key, value]; - } else if (value) { + if (value) { return [key, (value as any[]).map((u) => pickFields(keyConfig.config, u))]; } else { return [key, value]; @@ -130,7 +125,7 @@ export function isEmpty(value: any): boolean { */ export function areEqual(a?: T, b?: T, strictOrder?: boolean): boolean { if (isPlainObject(a)) { - return deepEquals(toJS(a), toJS(b)); + return deepEquals(a, b); } if (hasToJSON(a) || hasToJSON(b)) { const a1 = hasToJSON(a) ? a.toJSON() : a; @@ -151,9 +146,9 @@ export function hasToJSON(o?: unknown): o is { toJSON(): void } { return !!(o && typeof o === "object" && "toJSON" in o); } -/** Make a clone of `obj`, but only recurse into POJOs and Arrays...and stores. */ +/** Make a clone of `obj`, recurse into POJOs, Arrays, and class instances. */ export function deepClone(obj: T, map = new WeakMap()): T { - if (obj && typeof obj === "object" && (isPlainObject(obj) || Array.isArray(obj) || isObservable(obj))) { + if (obj && typeof obj === "object" && (isPlainObject(obj) || Array.isArray(obj) || isClassInstance(obj))) { if (map.has(obj)) return map.get(obj); const result = Array.isArray(obj) ? [] : {}; map.set(obj, result); @@ -176,7 +171,20 @@ export function groupBy(list: readonly T[], fn: (x: T) => return result; } -/** Returns all property names, including mobx computeds (non-enumerable) & inherited. */ +/** Returns true if the object is a class instance (not a plain object, Date, RegExp, etc.). */ +function isClassInstance(obj: unknown): boolean { + if (!obj || typeof obj !== "object") return false; + const proto = Object.getPrototypeOf(obj); + return ( + proto !== null && + proto !== Object.prototype && + !Array.isArray(obj) && + !(obj instanceof Date) && + !(obj instanceof RegExp) + ); +} + +/** Returns all property names, including non-enumerable & inherited. */ function getAllPropertyNames(obj: unknown): string[] { const proto = Object.getPrototypeOf(obj); // Don't crawl up into Object.prototype, or into arrays/observable arrays diff --git a/yarn.lock b/yarn.lock index 5f52666..9c1789f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -805,6 +805,7 @@ __metadata: resolution: "@homebound/form-state@workspace:." dependencies: "@homebound/rtl-utils": 2.66.2 + "@legendapp/state": 3.0.0-beta.46 "@semantic-release/exec": ^6.0.3 "@semantic-release/git": ^10.0.1 "@storybook/addon-essentials": ^8.3.5 @@ -836,8 +837,6 @@ __metadata: jest: ^29.7.0 jest-environment-jsdom: ^29.7.0 lint-staged: ^15.2.10 - mobx: ^6.13.5 - mobx-react: ^9.1.1 object-hash: ^3.0.0 prettier: 3.3.3 prettier-plugin-organize-imports: ^4.1.0 @@ -854,7 +853,7 @@ __metadata: vite: ^5.4.9 vite-tsconfig-paths: ^5.0.1 peerDependencies: - mobx: ">=6" + "@legendapp/state": ">=3" react: ">=16" languageName: unknown linkType: soft @@ -1237,6 +1236,20 @@ __metadata: languageName: node linkType: hard +"@legendapp/state@npm:3.0.0-beta.46": + version: 3.0.0-beta.46 + resolution: "@legendapp/state@npm:3.0.0-beta.46" + dependencies: + use-sync-external-store: ^1.2.2 + peerDependencies: + expo-sqlite: ^15.0.0 + peerDependenciesMeta: + expo-sqlite: + optional: true + checksum: 05d621bd7322a913e1c357052bc228c470079da2374ca7c3c40a8bf94eba3908d84858be55983bbe893c4415b8b9327a991ced2d573f4f3bdbd73b355f54dbbb + languageName: node + linkType: hard + "@mdx-js/react@npm:^3.0.0": version: 3.0.1 resolution: "@mdx-js/react@npm:3.0.1" @@ -8899,47 +8912,6 @@ __metadata: languageName: node linkType: hard -"mobx-react-lite@npm:^4.0.7": - version: 4.0.7 - resolution: "mobx-react-lite@npm:4.0.7" - dependencies: - use-sync-external-store: ^1.2.0 - peerDependencies: - mobx: ^6.9.0 - react: ^16.8.0 || ^17 || ^18 - peerDependenciesMeta: - react-dom: - optional: true - react-native: - optional: true - checksum: 8f49844d31116dc9b412b763d068f374014d87aee8e0c7aff93c85cd9df05e62027c5097fb7212527cda6f62114e288a048906b1d35e93138ebba43d885c349c - languageName: node - linkType: hard - -"mobx-react@npm:^9.1.1": - version: 9.1.1 - resolution: "mobx-react@npm:9.1.1" - dependencies: - mobx-react-lite: ^4.0.7 - peerDependencies: - mobx: ^6.9.0 - react: ^16.8.0 || ^17 || ^18 - peerDependenciesMeta: - react-dom: - optional: true - react-native: - optional: true - checksum: 2f64a6d5ac0653cc48cfd1270ec5b4cdcf4e2ad62e4ba9d876a625bdcdfe75613920497bac0d14306b45773e0b306ad3c2c47f2e34be8ef5e906efa170d3822e - languageName: node - linkType: hard - -"mobx@npm:^6.13.5": - version: 6.13.5 - resolution: "mobx@npm:6.13.5" - checksum: 2a253e505900169326873b573660dab58ce8284a435c75d77775a665af9eed623c1976a5eab3cfb53561cac4713f70194f49a2b8e4111419a2e4291f51e5485a - languageName: node - linkType: hard - "ms@npm:2.0.0": version: 2.0.0 resolution: "ms@npm:2.0.0" @@ -12387,12 +12359,12 @@ __metadata: languageName: node linkType: hard -"use-sync-external-store@npm:^1.2.0": - version: 1.2.2 - resolution: "use-sync-external-store@npm:1.2.2" +"use-sync-external-store@npm:^1.2.2": + version: 1.6.0 + resolution: "use-sync-external-store@npm:1.6.0" peerDependencies: - react: ^16.8.0 || ^17.0.0 || ^18.0.0 - checksum: fe07c071c4da3645f112c38c0e57beb479a8838616ff4e92598256ecce527f2888c08febc7f9b2f0ce2f0e18540ba3cde41eb2035e4fafcb4f52955037098a81 + react: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 + checksum: 61a62e910713adfaf91bdb72ff2cd30e5ba83687accaf3b6e75a903b45bf635f5722e3694af30d83a03e92cb533c0a5c699298d2fef639a03ffc86b469f4eee2 languageName: node linkType: hard