From 712ab005d34ab7a9bb4952c9df21d5d2c99050b2 Mon Sep 17 00:00:00 2001 From: Adrian Mucha Date: Wed, 29 May 2024 12:01:20 +0200 Subject: [PATCH 1/7] Add Bridge.getInitialModel --- packages/uniforms/__tests__/AutoForm.tsx | 69 ++++++++++++++++++++---- packages/uniforms/src/AutoForm.tsx | 21 ++++++-- packages/uniforms/src/Bridge.ts | 19 +++++++ 3 files changed, 96 insertions(+), 13 deletions(-) diff --git a/packages/uniforms/__tests__/AutoForm.tsx b/packages/uniforms/__tests__/AutoForm.tsx index 0d7d34b0b..29f54f350 100644 --- a/packages/uniforms/__tests__/AutoForm.tsx +++ b/packages/uniforms/__tests__/AutoForm.tsx @@ -39,7 +39,7 @@ describe('', () => { ); const input = screen.getByLabelText('A'); fireEvent.change(input, { target: { value: '2' } }); - expect(onChange).toHaveBeenCalledTimes(4); + expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenLastCalledWith('a', '2'); }); @@ -93,17 +93,63 @@ describe('', () => { schemaDefinition, ); + const fieldA = screen.getByLabelText('A'); + const fieldC = screen.getByLabelText('C'); + fireEvent.change(fieldA, { target: { value: 'a' } }); + fireEvent.change(fieldC, { target: { value: 'c' } }); + expect(contextSpy).toHaveBeenLastCalledWith( expect.objectContaining({ changed: true, - changedMap: { a: {}, b: {}, c: {} }, + changedMap: { a: {}, b: undefined, c: {} }, }), ); }); }); - describe('when render', () => { - it('calls `onChange` before render', () => { + describe('when initial render', () => { + it('merges initial values of schema and props.model', () => { + const { rerenderWithProps } = render( + + + + , + schemaDefinition, + ); + + // initial render shouldn't mark form as changed + expect(contextSpy).toHaveBeenLastCalledWith( + expect.objectContaining({ model: { a: '', b: '', c: 'c', d: 'd' } }), + ); + + rerenderWithProps({ model: { c: undefined, d: 'dd', e: 'ee' } }); + + expect(contextSpy).toHaveBeenLastCalledWith( + expect.objectContaining({ + model: { a: '', b: '', d: 'dd', e: 'ee' }, + }), + ); + }); + + it('keeps `changed` false', () => { + render( + + + + , + schemaDefinition, + ); + + // initial render shouldn't mark form as changed + expect(contextSpy).toHaveBeenLastCalledWith( + expect.objectContaining({ + changed: false, + changedMap: {}, + }), + ); + }); + + it('does not call `onChange`', () => { const field = jest.fn(() => null); const Field = connectField(field); @@ -123,12 +169,12 @@ describe('', () => { , ); - expect(onChange).toHaveBeenCalledTimes(2); - expect(onChange.mock.calls[0]).toEqual(expect.arrayContaining(['b', ''])); - expect(onChange.mock.calls[1]).toEqual(expect.arrayContaining(['c', ''])); + expect(onChange).toHaveBeenCalledTimes(0); expect(field).toHaveBeenCalled(); }); + }); + describe('when render', () => { it('skips `onSubmit` until rendered (`autosave` = true)', async () => { render( @@ -234,7 +280,7 @@ describe('', () => { }); }); - describe('when update', () => { + describe('when `props.model` update', () => { it(', updates', () => { const { rerenderWithProps } = render( @@ -243,9 +289,12 @@ describe('', () => { schemaDefinition, ); - rerenderWithProps({ model: {} }); expect(contextSpy).toHaveBeenLastCalledWith( - expect.objectContaining({ model: {} }), + expect.objectContaining({ model: { a: '', b: '', c: '' } }), + ); + rerenderWithProps({ model: { d: '123' } }); + expect(contextSpy).toHaveBeenLastCalledWith( + expect.objectContaining({ model: { a: '', b: '', c: '', d: '123' } }), ); }); diff --git a/packages/uniforms/src/AutoForm.tsx b/packages/uniforms/src/AutoForm.tsx index 0905d273f..0462b2188 100644 --- a/packages/uniforms/src/AutoForm.tsx +++ b/packages/uniforms/src/AutoForm.tsx @@ -38,14 +38,29 @@ export function Auto(Base: Base) { this.state = { ...this.state, - model: props.model, + model: this.mergeSchemaAndPropsModel( + this.props.schema, + this.props.model, + ), }; } + /** + * Returns model value based on the `schema` model and `props.model`. + * Latter one takes precedence. Does shallow copy. + */ + mergeSchemaAndPropsModel( + schema: Props['schema'], + model: Props['model'], + ): Props['model'] { + const initialModel = schema.getInitialModel(); + return Object.assign(initialModel, model); + } + componentDidUpdate(prevProps: Props, prevState: State, snapshot: never) { - const { model } = this.props; + const { model, schema } = this.props; if (!isEqual(model, prevProps.model)) { - this.setState({ model }); + this.setState({ model: this.mergeSchemaAndPropsModel(schema, model) }); } super.componentDidUpdate(prevProps, prevState, snapshot); diff --git a/packages/uniforms/src/Bridge.ts b/packages/uniforms/src/Bridge.ts index 5479f17dd..88577d0f1 100644 --- a/packages/uniforms/src/Bridge.ts +++ b/packages/uniforms/src/Bridge.ts @@ -1,5 +1,7 @@ import invariant from 'invariant'; +import set from 'lodash/set'; +import { joinName } from './joinName'; import { UnknownObject } from './types'; export abstract class Bridge { @@ -84,6 +86,23 @@ export abstract class Bridge { ); } + /** + * Get initial model value recursively. + */ + getInitialModel(): UnknownObject { + const initialModel: UnknownObject = {}; + const subFields = this.getSubfields(); + for (const fieldName of subFields) { + const initialValue = this.getInitialValue(fieldName); + set(initialModel, fieldName, initialValue); + const newSubFields = this.getSubfields(fieldName).map(subField => + joinName(fieldName, subField), + ); + subFields.push(...newSubFields); + } + return initialModel; + } + /** * Get props defined in schema for a field `name`. There are no required nor * banned fields, however properties like `required` are often available. From 1ee3f9d20a04f3cf8f955e1a20fdd64e09c0f5f5 Mon Sep 17 00:00:00 2001 From: Adrian Mucha Date: Tue, 4 Jun 2024 13:02:49 +0200 Subject: [PATCH 2/7] Run onChange(initialValue) always --- packages/uniforms/src/useField.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/uniforms/src/useField.tsx b/packages/uniforms/src/useField.tsx index 735dcf6af..6df717a37 100644 --- a/packages/uniforms/src/useField.tsx +++ b/packages/uniforms/src/useField.tsx @@ -65,8 +65,7 @@ export function useField< } // eslint-disable-next-line react-hooks/rules-of-hooks useEffect(() => { - const required = props.required ?? schemaProps.required; - if (required && initialValue !== undefined) { + if (initialValue !== undefined) { onChange(initialValue); } // eslint-disable-next-line react-hooks/exhaustive-deps From ecbdf3671a1d0c4008ff0b475eba04ea92a57efe Mon Sep 17 00:00:00 2001 From: Adrian Mucha Date: Wed, 5 Jun 2024 09:51:33 +0200 Subject: [PATCH 3/7] Fix duplicated fields --- packages/uniforms/src/Bridge.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/uniforms/src/Bridge.ts b/packages/uniforms/src/Bridge.ts index 88577d0f1..b65cd83e6 100644 --- a/packages/uniforms/src/Bridge.ts +++ b/packages/uniforms/src/Bridge.ts @@ -1,7 +1,6 @@ import invariant from 'invariant'; import set from 'lodash/set'; -import { joinName } from './joinName'; import { UnknownObject } from './types'; export abstract class Bridge { @@ -95,10 +94,6 @@ export abstract class Bridge { for (const fieldName of subFields) { const initialValue = this.getInitialValue(fieldName); set(initialModel, fieldName, initialValue); - const newSubFields = this.getSubfields(fieldName).map(subField => - joinName(fieldName, subField), - ); - subFields.push(...newSubFields); } return initialModel; } From 391938565466a3e547e694176de41beb86988b09 Mon Sep 17 00:00:00 2001 From: Adrian Mucha Date: Wed, 5 Jun 2024 11:14:22 +0200 Subject: [PATCH 4/7] Fix tests --- packages/uniforms-unstyled/__tests__/index.ts | 2 +- packages/uniforms/__suites__/ListField.tsx | 10 +++- packages/uniforms/__suites__/render.tsx | 39 ++++-------- packages/uniforms/__tests__/AutoForm.tsx | 6 -- packages/uniforms/__tests__/ValidatedForm.tsx | 14 +---- packages/uniforms/__tests__/connectField.tsx | 59 ++++++++++++------- 6 files changed, 61 insertions(+), 69 deletions(-) diff --git a/packages/uniforms-unstyled/__tests__/index.ts b/packages/uniforms-unstyled/__tests__/index.ts index 097876b9d..2db15f1f8 100644 --- a/packages/uniforms-unstyled/__tests__/index.ts +++ b/packages/uniforms-unstyled/__tests__/index.ts @@ -44,7 +44,7 @@ describe('@RTL', () => { suites.testListAddField(theme.ListAddField); suites.testListDelField(theme.ListDelField); suites.testListField(theme.ListField, { - getListAddField: screen => screen.getByRole('button'), + getListAddField: screen => screen.getByRole('button', { name: '+' }), testError: false, }); suites.testListItemField(theme.ListItemField); diff --git a/packages/uniforms/__suites__/ListField.tsx b/packages/uniforms/__suites__/ListField.tsx index 3c534102d..3b88c26e1 100644 --- a/packages/uniforms/__suites__/ListField.tsx +++ b/packages/uniforms/__suites__/ListField.tsx @@ -179,13 +179,17 @@ export function testListField( const onChange = jest.fn(); render( , - { x: { type: Array, optional: true }, 'x.$': String }, + { + x: { type: Array, optional: true }, + 'x.$': { type: Object }, + 'x.$.name': { type: String, defaultValue: 'kebab' }, + }, { onChange }, ); await userEvent.click(getListAddField(screen)); - expect(onChange).toHaveBeenCalledTimes(1); - expect(onChange).toHaveBeenLastCalledWith('x', [undefined]); + expect(await screen.findAllByDisplayValue('kebab')).toHaveLength(1); + expect(onChange).toHaveBeenLastCalledWith('x.0', { name: 'kebab' }); }); if (testError) { diff --git a/packages/uniforms/__suites__/render.tsx b/packages/uniforms/__suites__/render.tsx index 0f2d54985..14aac7b4d 100644 --- a/packages/uniforms/__suites__/render.tsx +++ b/packages/uniforms/__suites__/render.tsx @@ -1,43 +1,30 @@ import { render as renderOnScreen, RenderResult } from '@testing-library/react'; import React, { ReactElement, cloneElement } from 'react'; import SimpleSchema, { SimpleSchemaDefinition } from 'simpl-schema'; -import { BaseForm, Context, UnknownObject, context, randomIds } from 'uniforms'; +import { Context, UnknownObject } from 'uniforms'; import { SimpleSchema2Bridge } from 'uniforms-bridge-simple-schema-2'; +import { AutoForm } from 'uniforms-unstyled'; -const randomId = randomIds(); export function render( element: ReactElement

, schema?: SimpleSchemaDefinition, - contextValueExtension?: Partial>, + contextValueExtension?: Pick>, 'onChange'>, model = {} as Model, ): RenderResult & { rerenderWithProps: (props: P) => void } { const renderResult = renderOnScreen(element, { wrapper({ children }) { if (schema) { - const contextValue = { - changed: false, - changedMap: {}, - error: null, - model, - name: [], - onChange() {}, - onSubmit() {}, - randomId, - submitted: false, - submitting: false, - validating: false, - ...contextValueExtension, - schema: new SimpleSchema2Bridge({ schema: new SimpleSchema(schema) }), - state: { - disabled: false, - readOnly: false, - showInlineError: false, - ...contextValueExtension?.state, - }, - formRef: {} as BaseForm, - }; + const bridge = new SimpleSchema2Bridge({ + schema: new SimpleSchema(schema), + }); return ( - {children} + + {children} + ); } return <>{children}; diff --git a/packages/uniforms/__tests__/AutoForm.tsx b/packages/uniforms/__tests__/AutoForm.tsx index 29f54f350..86355ba75 100644 --- a/packages/uniforms/__tests__/AutoForm.tsx +++ b/packages/uniforms/__tests__/AutoForm.tsx @@ -34,8 +34,6 @@ describe('', () => { , - schemaDefinition, - { onChange }, ); const input = screen.getByLabelText('A'); fireEvent.change(input, { target: { value: '2' } }); @@ -90,7 +88,6 @@ describe('', () => { , - schemaDefinition, ); const fieldA = screen.getByLabelText('A'); @@ -114,7 +111,6 @@ describe('', () => { , - schemaDefinition, ); // initial render shouldn't mark form as changed @@ -137,7 +133,6 @@ describe('', () => { , - schemaDefinition, ); // initial render shouldn't mark form as changed @@ -286,7 +281,6 @@ describe('', () => { , - schemaDefinition, ); expect(contextSpy).toHaveBeenLastCalledWith( diff --git a/packages/uniforms/__tests__/ValidatedForm.tsx b/packages/uniforms/__tests__/ValidatedForm.tsx index 7c182ad5e..a5b594cd7 100644 --- a/packages/uniforms/__tests__/ValidatedForm.tsx +++ b/packages/uniforms/__tests__/ValidatedForm.tsx @@ -150,7 +150,6 @@ describe('ValidatedForm', () => { > , - schemaDefinition, ); const form = screen.getByRole('form'); @@ -173,7 +172,6 @@ describe('ValidatedForm', () => { > , - schemaDefinition, ); const form = screen.getByRole('form'); @@ -199,7 +197,6 @@ describe('ValidatedForm', () => { > , - schemaDefinition, ); expect(contextSpy).toHaveBeenCalledWith( @@ -286,7 +283,6 @@ describe('ValidatedForm', () => { > , - schemaDefinition, ); const form = screen.getByRole('form'); @@ -314,7 +310,6 @@ describe('ValidatedForm', () => { > , - schemaDefinition, ); validator.mockImplementationOnce(() => { @@ -347,7 +342,6 @@ describe('ValidatedForm', () => { > , - schemaDefinition, ); onSubmit.mockImplementationOnce(() => Promise.reject(error)); @@ -485,7 +479,6 @@ describe('ValidatedForm', () => { , - schemaDefinition, ); validator.mockImplementationOnce(() => { throw error; @@ -555,20 +548,20 @@ describe('ValidatedForm', () => { ); it('does not revalidate when `model` changes', () => { - const { rerenderWithProps } = render(, schemaDefinition); + const { rerenderWithProps } = render(); rerenderWithProps({ model: {} }); expect(validator).not.toBeCalled(); }); it('does not revalidate when validator `options` change', () => { - const { rerenderWithProps } = render(, schemaDefinition); + const { rerenderWithProps } = render(); rerenderWithProps({ validator: {} }); expect(validator).not.toBeCalled(); }); it('does not revalidate when `schema` changes', () => { const anotherSchema = new SimpleSchema2Bridge({ schema: schema.schema }); - const { rerenderWithProps } = render(, schemaDefinition); + const { rerenderWithProps } = render(); rerenderWithProps({ schema: anotherSchema }); expect(validator).not.toBeCalled(); }); @@ -721,7 +714,6 @@ describe('ValidatedForm', () => { > , - schemaDefinition, ); const asyncSubmission = onSubmitMode.includes('async'); diff --git a/packages/uniforms/__tests__/connectField.tsx b/packages/uniforms/__tests__/connectField.tsx index 6865b8fff..82c14cb68 100644 --- a/packages/uniforms/__tests__/connectField.tsx +++ b/packages/uniforms/__tests__/connectField.tsx @@ -20,6 +20,7 @@ describe('connectField', () => { another: { type: String, optional: true }, field: { type: Object, label: 'Field' }, 'field.subfield': { type: String, label: 'Subfield' }, + fieldWithDefaultValue: { type: String, defaultValue: 'John' }, }; const reactContext = { @@ -47,9 +48,11 @@ describe('connectField', () => { onChange: OnChange; label?: string | React.ReactNode; id: string; + value: string | number; }, ) => { - return props.children ? ( + const filteredProps = filterDOMProps(omit(props, 'children', 'label')); + return ( <> {props.label && (