From eb3970f060f31238868f7f59331c481a56ea4303 Mon Sep 17 00:00:00 2001 From: Yakov Zhmurov Date: Fri, 6 Dec 2024 13:38:28 +0300 Subject: [PATCH] useForm - partially refactored to switch from useRef to useState. Lenses are still an issue. --- .../src/data/forms/__tests__/useForm.test.tsx | 53 +++++ uui-core/src/data/forms/useForm.ts | 197 +++++++++--------- uui-core/src/data/lenses/LensBuilder.ts | 64 ++++-- uui-core/src/data/lenses/builders.ts | 18 +- uui-core/src/data/lenses/types.ts | 12 +- 5 files changed, 211 insertions(+), 133 deletions(-) diff --git a/uui-core/src/data/forms/__tests__/useForm.test.tsx b/uui-core/src/data/forms/__tests__/useForm.test.tsx index dddd5f962b..0add2eb4d1 100644 --- a/uui-core/src/data/forms/__tests__/useForm.test.tsx +++ b/uui-core/src/data/forms/__tests__/useForm.test.tsx @@ -86,6 +86,59 @@ describe('useForm', () => { expect(result.current.isInvalid).toBe(false); }); + it('Should update form value with lens.update (2 immediate updates)', async () => { + const { result } = await renderHookWithContextAsync, IFormApi>(() => + useForm({ + onSave: () => Promise.resolve(), + onError: () => Promise.resolve(), + value: 1, + })); + + act(() => { + result.current.lens.update((x) => x + 1); + result.current.lens.update((x) => x + 1); + }); + expect(result.current.value).toBe(3); + expect(result.current.isChanged).toBe(true); + expect(result.current.isInvalid).toBe(false); + }); + + it('Should get actual form value form value w/o re-render.', async () => { + const { result } = await renderHookWithContextAsync, IFormApi>(() => + useForm({ + onSave: () => Promise.resolve(), + onError: () => Promise.resolve(), + value: 1, + })); + + act(() => { + result.current.lens.set(2); + expect(result.current.lens.get()).toBe(2); + result.current.lens.set(3); + }); + expect(result.current.value).toBe(3); + expect(result.current.isChanged).toBe(true); + expect(result.current.isInvalid).toBe(false); + }); + + it('Should update via lens.toProps()', async () => { + const { result } = await renderHookWithContextAsync, IFormApi>(() => + useForm({ + onSave: () => Promise.resolve(), + onError: () => Promise.resolve(), + value: 1, + })); + + act(() => { + const props = result.current.lens.toProps(); + props.onValueChange(2); + props.onValueChange(3); + }); + expect(result.current.value).toBe(3); + expect(result.current.isChanged).toBe(true); + expect(result.current.isInvalid).toBe(false); + }); + it('should update form value by external props.value change', async () => { const { result, rerender } = await renderHookWithContextAsync, IFormApi>((props) => useForm(props), { onSave: () => Promise.resolve(), diff --git a/uui-core/src/data/forms/useForm.ts b/uui-core/src/data/forms/useForm.ts index b331d64855..ae27fd9694 100644 --- a/uui-core/src/data/forms/useForm.ts +++ b/uui-core/src/data/forms/useForm.ts @@ -17,6 +17,7 @@ import { flushSync } from 'react-dom'; interface FormState { form: T; + initialForm: T; validationState: ValidationState; serverValidationState: ValidationState; lastSentForm?: T; @@ -27,52 +28,51 @@ interface FormState { isInSaveMode: boolean; } -export type UseFormProps = Omit, 'renderForm'>; - -export function useForm(props: UseFormProps): IFormApi { - const context: UuiContexts = useUuiContext(); - - const initialForm = useRef>({ +function getInitialFormState(value: T) { + return { isChanged: false, isInProgress: false, - form: props.value, + form: value, + initialForm: value, validationState: { isInvalid: false }, serverValidationState: undefined, - formHistory: [props.value], + formHistory: [value], historyIndex: 0, isInSaveMode: false, - }); + } as FormState; +} - const propsRef = useRef(props); - propsRef.current = props; +export type UseFormProps = Omit, 'renderForm'>; - const getMetadata = (value: T) => - propsRef.current.getMetadata ? propsRef.current.getMetadata(value) : {}; +export function useForm(props: UseFormProps): IFormApi { + const context: UuiContexts = useUuiContext(); - const prevFormValue = useRef(props.value); + const propsRef = useRef(props); + propsRef.current = props; - const formState = useRef(initialForm.current); + const formStateRef = useRef(getInitialFormState(props.value)); + const formState = formStateRef.current; const forceUpdate = useForceUpdate(); - const updateFormState = ( + const setFormState = ( update: (current: FormState) => FormState, ) => { - const newState = update(formState.current); - formState.current = newState; + const newState = update(formStateRef.current); + formStateRef.current = newState; forceUpdate(); }; const handleSave = useCallback((isSavedBeforeLeave?: boolean) => { let savePromise: any; - updateFormState((currentState) => { + setFormState((currentState) => { let newState = { ...currentState, isInSaveMode: true }; newState.isInSaveMode = true; newState = updateValidationStates(newState); if (!newState.validationState.isInvalid) { newState.isInProgress = true; savePromise = propsRef.current - .onSave(formState.current.form) + .onSave(newState.form) .then((response) => handleSaveResponse(response, isSavedBeforeLeave)) .catch((err) => handleError(err)); @@ -111,47 +111,22 @@ export function useForm(props: UseFormProps): IFormApi { const { isLocked, block, unblock } = useLock({ handleLeave }); - const getMergedValidationState = () => { - const { - form, lastSentForm, serverValidationState, validationState, - } = formState.current; - if (serverValidationState) { - const serverValidation = validateServerErrorState(form, lastSentForm, serverValidationState); - return mergeValidation(validationState, serverValidation); - } - return validationState; - }; - - const lens = useMemo( - () => - new LensBuilder({ - get: () => formState.current.form, - set: (_, small: T) => { - handleFormUpdate(() => small); - return small; - }, - getValidationState: getMergedValidationState, - getMetadata: () => getMetadata(formState.current.form), - }), - [], - ); - useEffect(() => { const unsavedChanges = getUnsavedChanges(); - if (!unsavedChanges || !props.loadUnsavedChanges || isEqual(unsavedChanges, initialForm.current.form)) return; + if (!unsavedChanges || !props.loadUnsavedChanges || isEqual(unsavedChanges, formState.initialForm)) return; props .loadUnsavedChanges() .then(() => handleFormUpdate(() => unsavedChanges)) .catch(() => null); }, []); + // Store props.value from the previous render + const prevFormValue = useRef(props.value); + + // Reset form if props.value has changed since the previous render useEffect(() => { if (!isEqual(props.value, prevFormValue.current)) { - resetForm({ - ...formState.current, - form: props.value, - formHistory: formState.current.isChanged ? formState.current.formHistory : [props.value], - }); + setFormState(() => getInitialFormState(props.value)); prevFormValue.current = props.value; } }, [props.value]); @@ -160,9 +135,8 @@ export function useForm(props: UseFormProps): IFormApi { return context.uuiUserSettings.get(props.settingsKey); }; - const handleFormUpdate = (update: (current: T) => T, options?: { addCheckpoint?: boolean }) => - updateFormState((currentState) => { - options = options ?? {}; + const handleFormUpdate = (update: (current: T) => T, options: { addCheckpoint?: boolean } = {}) => + setFormState((currentState) => { options.addCheckpoint = options.addCheckpoint ?? true; const newForm = update(currentState.form); @@ -182,7 +156,7 @@ export function useForm(props: UseFormProps): IFormApi { context.uuiUserSettings.set(props.settingsKey, newForm); } - const isChanged = !isEqual(initialForm.current.form, newForm); + const isChanged = !isEqual(currentState.initialForm, newForm); if (isChanged === true) { block(); @@ -205,21 +179,15 @@ export function useForm(props: UseFormProps): IFormApi { return newState; }); - const resetForm = (withNewState: FormState) => - updateFormState((currentState) => { - const newFormState = { ...currentState, ...withNewState }; - if (newFormState !== currentState) { - initialForm.current = newFormState; - return newFormState; - } - }); + const getMetadata = (value: T) => + propsRef.current.getMetadata ? propsRef.current.getMetadata(value) : {}; const updateValidationStates = (state: FormState): FormState => { const valueToValidate = state.form; const metadata = getMetadata(valueToValidate); const isInSaveMode = state.isInSaveMode; const validationMode = isInSaveMode || !props.validationOn ? 'save' : props.validationOn; - const validationState = uuiValidate(valueToValidate, metadata, initialForm.current.form, validationMode); + const validationState = uuiValidate(valueToValidate, metadata, state.initialForm, validationMode); const newState = { ...state, validationState }; @@ -231,7 +199,7 @@ export function useForm(props: UseFormProps): IFormApi { }; const handleError = (err?: any) => { - updateFormState((currentValue) => ({ + setFormState((currentValue) => ({ ...currentValue, isInProgress: false, })); @@ -240,28 +208,28 @@ export function useForm(props: UseFormProps): IFormApi { }; const handleSaveResponse = (response: FormSaveResponse | void, isSavedBeforeLeave?: boolean) => { - const newFormValue = (response && response.form) || formState.current.form; - const newState: FormState = { - ...formState.current, - historyIndex: 0, - formHistory: [newFormValue], - isChanged: response && response.validation?.isInvalid ? formState.current.isChanged : false, - form: newFormValue, - isInProgress: false, - serverValidationState: (response && response.validation) || formState.current.serverValidationState, - lastSentForm: response && response.validation?.isInvalid ? response.form || formState.current.form : formState.current.lastSentForm, - }; - if (response && response.validation) { - flushSync(() => { - updateFormState(() => newState); - }); - return; - } - flushSync(() => { - resetForm(newState); - removeUnsavedChanges(); - unblock(); - }); + flushSync(() => setFormState((currentState) => { + const newFormValue = (response && response.form) || currentState.form; + const isServerInvalid = response && response.validation && response.validation.isInvalid; + const newState: FormState = { + ...currentState, + historyIndex: 0, + formHistory: [newFormValue], + isChanged: isServerInvalid ? currentState.isChanged : false, + form: newFormValue, + isInProgress: false, + serverValidationState: (response && response.validation) || currentState.serverValidationState, + lastSentForm: response && response.validation?.isInvalid ? response.form || currentState.form : currentState.lastSentForm, + }; + + if (!isServerInvalid) { + newState.initialForm = newFormValue; + unblock(); + removeUnsavedChanges(); + } + + return newState; + })); if (propsRef.current.onSuccess && response) { propsRef.current.onSuccess(response.form, isSavedBeforeLeave); @@ -270,7 +238,7 @@ export function useForm(props: UseFormProps): IFormApi { const handleUndo = useCallback( () => - updateFormState((currentState) => { + setFormState((currentState) => { const { formHistory, historyIndex } = currentState; const previousIndex = historyIndex - 1; @@ -295,7 +263,7 @@ export function useForm(props: UseFormProps): IFormApi { const handleRedo = useCallback( () => - updateFormState((currentState) => { + setFormState((currentState) => { const { formHistory, historyIndex } = currentState; const nextIndex = historyIndex + 1; if (nextIndex < currentState.formHistory.length) { @@ -315,15 +283,15 @@ export function useForm(props: UseFormProps): IFormApi { ); const validate = useCallback(() => { - const formSate = { ...formState.current, isInSaveMode: true }; + const formSate = { ...formState, isInSaveMode: true }; const newState = updateValidationStates(formSate); - updateFormState(() => newState); + setFormState(() => newState); return newState.validationState; }, []); const handleRevert = useCallback(() => { - resetForm(initialForm.current); + setFormState((current) => getInitialFormState(current.initialForm)); }, [props.value]); const handleValueChange = useCallback((newValue: T) => { @@ -338,7 +306,7 @@ export function useForm(props: UseFormProps): IFormApi { }, []); const handleReplaceValue = useCallback((value: React.SetStateAction) => { - updateFormState((currentValue) => { + setFormState((currentValue) => { const newFormValue = value instanceof Function ? value(currentValue.form) : value; return { ...currentValue, @@ -356,7 +324,7 @@ export function useForm(props: UseFormProps): IFormApi { }, [isLocked]); const setServerValidationState = useCallback((value: React.SetStateAction) => { - updateFormState((currentValue) => { + setFormState((currentValue) => { const newValue = value instanceof Function ? value(currentValue.serverValidationState) : value; return { ...currentValue, @@ -365,12 +333,37 @@ export function useForm(props: UseFormProps): IFormApi { }); }, []); - const mergedValidationState = getMergedValidationState(); + const getMergedValidationState = (state: FormState) => { + const { + form, lastSentForm, serverValidationState, validationState, + } = state; + if (serverValidationState) { + const serverValidation = validateServerErrorState(form, lastSentForm, serverValidationState); + return mergeValidation(validationState, serverValidation); + } + return validationState; + }; + + // Build lens. + // We still need to use formStateRef.current instead of formState, + // More on this here: https://github.com/epam/UUI/issues/2668 + const lens = useMemo( + () => + new LensBuilder({ + get: () => formStateRef.current.form, + set: handleFormUpdate, + getValidationState: () => getMergedValidationState(formStateRef.current), + getMetadata: () => getMetadata(formStateRef.current.form), + }), + [], + ); + + const mergedValidationState = getMergedValidationState(formState); return { setValue: handleSetValue, replaceValue: handleReplaceValue, - isChanged: formState.current.isChanged, + isChanged: formState.isChanged, close: handleClose, lens, save: saveCallback, @@ -378,16 +371,16 @@ export function useForm(props: UseFormProps): IFormApi { redo: handleRedo, revert: handleRevert, validate, - canUndo: formState.current.historyIndex > 0, - canRedo: formState.current.historyIndex < formState.current.formHistory.length - 1, - canRevert: formState.current.form !== props.value, - value: formState.current.form, + canUndo: formState.historyIndex > 0, + canRedo: formState.historyIndex < formState.formHistory.length - 1, + canRevert: formState.form !== props.value, + value: formState.form, onValueChange: handleValueChange, isInvalid: mergedValidationState.isInvalid, validationMessage: mergedValidationState.validationMessage, validationProps: mergedValidationState.validationProps, - serverValidationState: formState.current.serverValidationState, + serverValidationState: formState.serverValidationState, setServerValidationState, - isInProgress: formState.current.isInProgress, + isInProgress: formState.isInProgress, }; } diff --git a/uui-core/src/data/lenses/LensBuilder.ts b/uui-core/src/data/lenses/LensBuilder.ts index 94df7f5bae..ef6a1c0971 100644 --- a/uui-core/src/data/lenses/LensBuilder.ts +++ b/uui-core/src/data/lenses/LensBuilder.ts @@ -1,40 +1,53 @@ -import { IEditable } from '../../types'; +import { IEditable, Metadata } from '../../types'; import * as Impl from './lensesImpl'; import { ILensImpl } from './lensesImpl'; -import { ILens, ArrayElement, IMapElement } from './types'; +import { ILens, ArrayElement, IMapElement, ValidationState } from './types'; +export interface LensBuilderProps { + get(): T; + set(update: (current: T) => T): void; + getValidationState?(): ValidationState; + getMetadata?(): Metadata; +} export class LensBuilder implements ILens { - public readonly handleValueChange: (newValue: TFocused) => void = null; - constructor(public readonly lens: ILensImpl) { - this.handleValueChange = (newValue: TFocused) => { - this.lens.set(null, newValue); - }; + constructor( + private props: LensBuilderProps, + private lens: ILensImpl = Impl.identity() as any, + ) { } public get(): TFocused { - return this.lens.get(null); + const big = this.props.get(); + return this.lens.get(big); } - public key(id: TId): LensBuilder> { - return this.compose(Impl.key(id) as any, id); + public set(newValue: TFocused) { + this.props.set((current) => this.lens.set(current, newValue)); } - public set(value: TFocused) { - this.lens.set(null, value); - } + // onValueChange should be bound to 'this', as it is returned from .toProps() in a separate object + private handleOnValueChange = (newValue: TFocused) => this.set(newValue); public update(fn: (current: TFocused) => TFocused) { - this.lens.set(null, fn(this.lens.get(null))); + this.props.set((currentRoot) => { + const currentFocused = this.lens.get(currentRoot); + const updatedFocused = fn(currentFocused); + const updatedRoot = this.lens.set(currentRoot, updatedFocused); + return updatedRoot; + }); } + // We cache LensBuilder instances to not re-create handleOnValueChange, as it would break memoization + // in React.memo-wrapped components, which is especially critical for DataTableRow. public static MAX_CACHE_SIZE = 1000; private cache = new Map(); + public compose(lens: ILensImpl, cacheKey?: any): LensBuilder { if (cacheKey != null && this.cache.has(cacheKey)) { return this.cache.get(cacheKey); } - const result = new LensBuilder(Impl.compose(this.lens, lens)); + const result = new LensBuilder(this.props, Impl.compose(this.lens, lens)); if (cacheKey != null) { this.cache.set(cacheKey, result); @@ -52,13 +65,20 @@ export class LensBuilder implements ILens return this.compose(Impl.prop(name), name) as any; } + public key(id: TId): LensBuilder> { + return this.compose(Impl.key(id) as any, id); + } + public index(index: number): LensBuilder> { return this.compose(Impl.index(index) as any, index); } public onChange(fn: (oldValue: TFocused, newValue: TFocused) => TFocused): LensBuilder { return this.compose({ - get: (i) => i, set: fn, getValidationState: this.lens.getValidationState, getMetadata: this.lens.getMetadata as any, + get: (i) => i, + set: fn, + getValidationState: this.lens.getValidationState, + getMetadata: this.lens.getMetadata as any, }, fn); } @@ -67,11 +87,15 @@ export class LensBuilder implements ILens } public toProps(): IEditable { - const validationState = this.lens.getValidationState && this.lens.getValidationState(null); - const metadata = this.lens.getMetadata && this.lens.getMetadata(null); + const rootValue = this.props.get(); + const rootValidationsState = this.props.getValidationState?.() ?? {}; + const rootMetadata = this.props.getMetadata?.() ?? {}; + const value = this.lens.get(rootValue); + const validationState = this.lens.getValidationState && this.lens.getValidationState(rootValidationsState); + const metadata = this.lens.getMetadata && this.lens.getMetadata(rootMetadata); return { - value: this.lens.get(null), - onValueChange: this.handleValueChange, + value, + onValueChange: this.handleOnValueChange, ...validationState, ...metadata, }; diff --git a/uui-core/src/data/lenses/builders.ts b/uui-core/src/data/lenses/builders.ts index 2996e8602b..e6f40bae73 100644 --- a/uui-core/src/data/lenses/builders.ts +++ b/uui-core/src/data/lenses/builders.ts @@ -8,9 +8,9 @@ export function onEditableComponent(component: any): ILens { get() { return component.props.value; }, - set(big: any, small: any) { - component.props.onValueChange(small); - return small; + set(update) { + const newValue = update(component.props.value); + component.props.onValueChange(newValue); }, getValidationState() { const { isInvalid, validationMessage, validationProps } = component.props; @@ -32,9 +32,9 @@ export function onEditable(editable: IEditable): ILens { get() { return editable.value; }, - set(big: any, small: any) { - editable.onValueChange(small); - return small; + set(update) { + const newValue = update(editable.value); + editable.onValueChange(newValue); }, getValidationState() { return editable; @@ -48,9 +48,9 @@ export function onState(component: any): ILens { get() { return component.state; }, - set(_, small: any) { - component.setState(small); - return small; + set(update) { + const newValue = update(component.state); + component.setState(newValue); }, }); } diff --git a/uui-core/src/data/lenses/types.ts b/uui-core/src/data/lenses/types.ts index c3f16cc001..75a1e723b7 100644 --- a/uui-core/src/data/lenses/types.ts +++ b/uui-core/src/data/lenses/types.ts @@ -18,24 +18,32 @@ export interface ValidationState extends ICanBeInvalid, IHasValidationMessage { export interface ILens { /** Get lens value */ get(): TFocused; - /** Get lens value of the IMap or IImmutableMap by provided id. */ - key(id: TId): ILens>>; + /** Set new lens value */ set(value: TFocused): void; + /** Updates lens value with returned value from provided callback. * This callback received current lens value as a param * */ update(fn: (current: TFocused) => TFocused): void; + /** Return a new lens on the provided field name */ prop(name: K): ILens>; + /** Return a new lens on item of array by provided index */ index(index: number): ILens>; + + /** Get lens value of the IMap or IImmutableMap by provided id. */ + key(id: TId): ILens>>; + /** Add to the lens a setter callback, which received old and new value and should return new value for set. * This callback will be called on any lens update * */ onChange(fn: (oldValue: TFocused, newValue: TFocused) => TFocused): ILens; + /** Defines default lens value, which will be return in case of lens have 'null' or 'undefined' value */ default(value: TFocused): ILens; + /** Return IEditable interface, which accepted by UUI form components. * Usually you just need to spread it to the component, e.g. { ...lens.prop('name').toProps() } */ toProps(): IEditable & ValidationState;