From 74525c897588f80594621c0a674e53c31886e070 Mon Sep 17 00:00:00 2001 From: Krisjanis Seglins Date: Fri, 6 Sep 2024 12:54:19 +0300 Subject: [PATCH 1/6] Send react updates only when not rendering Fixes: #398 --- packages/ts/react-crud/test/autocrud.spec.tsx | 51 ++++++++++++++++++- packages/ts/react-form/src/index.ts | 13 ++++- packages/ts/react-form/test/useForm.spec.tsx | 2 +- 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/packages/ts/react-crud/test/autocrud.spec.tsx b/packages/ts/react-crud/test/autocrud.spec.tsx index 46db6b3732..3bcafbd131 100644 --- a/packages/ts/react-crud/test/autocrud.spec.tsx +++ b/packages/ts/react-crud/test/autocrud.spec.tsx @@ -1,8 +1,15 @@ // eslint-disable-next-line /// import { expect, use } from '@esm-bundle/chai'; -import { render, screen, waitForElementToBeRemoved, within } from '@testing-library/react'; +import { render, screen, waitFor, waitForElementToBeRemoved, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { + _getPropertyModel, + makeObjectEmptyValueCreator, + NumberModel, + ObjectModel, + StringModel, +} from '@vaadin/hilla-lit-form'; import { TextField } from '@vaadin/react-components/TextField.js'; import chaiDom from 'chai-dom'; import sinon from 'sinon'; @@ -12,7 +19,7 @@ import ConfirmDialogController from './ConfirmDialogController.js'; import { CrudController } from './CrudController.js'; import FormController from './FormController'; import GridController from './GridController'; -import { getItem, PersonModel, personService } from './test-models-and-services.js'; +import { createService, getItem, PersonModel, personService } from './test-models-and-services.js'; use(sinonChai); use(chaiDom); @@ -325,6 +332,46 @@ describe('@vaadin/hilla-react-crud', () => { expect((await form.getField('Last name')).value).to.equal(''); }); + it('opens the form in a dialog when row has undefined value', async () => { + type PData = { + id: number; + name?: number; + }; + const nullData: PData[] = [ + { + id: 1, + name: undefined, + }, + ]; + class PDataModel extends ObjectModel { + static override createEmptyValue = makeObjectEmptyValueCreator(PDataModel); + get id(): NumberModel { + return this[_getPropertyModel]( + 'id', + (parent, key) => + new NumberModel(parent, key, true, { + meta: { annotations: [{ name: 'jakarta.persistence.Id' }], javaType: 'java.lang.Long' }, + }), + ); + } + + get name(): StringModel { + return this[_getPropertyModel]( + 'name', + (parent, key) => new StringModel(parent, key, false, { meta: { javaType: 'java.lang.String' } }), + ); + } + } + const customService = createService(nullData); + const result = render(); + const grid = await GridController.init(result, user); + await grid.toggleRowSelected(0); + + const form = await FormController.init(user); + expect(form.instance).to.exist; + expect((await form.getField('Name')).value).to.equal(undefined); + }); + it('closes the dialog when clicking close button', async () => { const result = render(); const grid = await GridController.init(result, user); diff --git a/packages/ts/react-form/src/index.ts b/packages/ts/react-form/src/index.ts index 70a2b16368..b2c9a468f5 100644 --- a/packages/ts/react-form/src/index.ts +++ b/packages/ts/react-form/src/index.ts @@ -26,9 +26,16 @@ import type { Writable } from 'type-fest'; // eslint-disable-next-line @typescript-eslint/no-unsafe-call __REGISTER__(); +let isRendering = false; + function useUpdate() { - const [_, update] = useReducer((x: number) => x + 1, 0); - return update; + const [_, count] = useReducer((x: number) => x + 1, 0); + return () => { + if (isRendering) { + return; + } + count(); + }; } export type FieldDirectiveResult = Readonly<{ @@ -128,6 +135,7 @@ function useFields(node: BinderNode): FieldDirective const registry = new WeakMap(); return ((model: AbstractModel) => { + isRendering = true; const n = getBinderNode(model); let fieldState = registry.get(model); @@ -211,6 +219,7 @@ function useFields(node: BinderNode): FieldDirective fieldState.strategy.invalid = n.invalid; } + isRendering = false; return { name: n.name, ref: fieldState.ref, diff --git a/packages/ts/react-form/test/useForm.spec.tsx b/packages/ts/react-form/test/useForm.spec.tsx index 09361f65e3..ab46fbcee3 100644 --- a/packages/ts/react-form/test/useForm.spec.tsx +++ b/packages/ts/react-form/test/useForm.spec.tsx @@ -6,7 +6,7 @@ import chaiDom from 'chai-dom'; import { useEffect, useState } from 'react'; import sinon from 'sinon'; import sinonChai from 'sinon-chai'; -import { useForm as _useForm, useFormArrayPart, useFormPart } from '../src/index.js'; +import { useForm as _useForm, useFormArrayPart, useFormPart } from '../src'; import { type Contract, EntityModel, From 1df21862f29b5c52cfa510bc66b2255d030caaa3 Mon Sep 17 00:00:00 2001 From: Krisjanis Seglins Date: Fri, 6 Sep 2024 16:37:37 +0300 Subject: [PATCH 2/6] Remove redundant import --- packages/ts/react-crud/test/autocrud.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ts/react-crud/test/autocrud.spec.tsx b/packages/ts/react-crud/test/autocrud.spec.tsx index 3bcafbd131..6b342505ba 100644 --- a/packages/ts/react-crud/test/autocrud.spec.tsx +++ b/packages/ts/react-crud/test/autocrud.spec.tsx @@ -1,7 +1,7 @@ // eslint-disable-next-line /// import { expect, use } from '@esm-bundle/chai'; -import { render, screen, waitFor, waitForElementToBeRemoved, within } from '@testing-library/react'; +import { render, screen, waitForElementToBeRemoved, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { _getPropertyModel, From 19774f5da80b63c0aa2bc61e4b3aafd88106cba6 Mon Sep 17 00:00:00 2001 From: Krisjanis Seglins Date: Fri, 6 Sep 2024 16:55:10 +0300 Subject: [PATCH 3/6] Remove redundant import --- packages/ts/react-crud/test/autocrud.spec.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/ts/react-crud/test/autocrud.spec.tsx b/packages/ts/react-crud/test/autocrud.spec.tsx index 6b342505ba..e73765d2f1 100644 --- a/packages/ts/react-crud/test/autocrud.spec.tsx +++ b/packages/ts/react-crud/test/autocrud.spec.tsx @@ -356,10 +356,7 @@ describe('@vaadin/hilla-react-crud', () => { } get name(): StringModel { - return this[_getPropertyModel]( - 'name', - (parent, key) => new StringModel(parent, key, false, { meta: { javaType: 'java.lang.String' } }), - ); + return this[_getPropertyModel]('name', (parent, key) => new StringModel(parent, key, false)); } } const customService = createService(nullData); From e860bedd60be200ca55a023e9175f4e94e599c63 Mon Sep 17 00:00:00 2001 From: Krisjanis Seglins Date: Mon, 9 Sep 2024 10:35:39 +0300 Subject: [PATCH 4/6] Make property optional --- packages/ts/react-crud/test/autocrud.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ts/react-crud/test/autocrud.spec.tsx b/packages/ts/react-crud/test/autocrud.spec.tsx index e73765d2f1..39a9ed6dd1 100644 --- a/packages/ts/react-crud/test/autocrud.spec.tsx +++ b/packages/ts/react-crud/test/autocrud.spec.tsx @@ -356,7 +356,7 @@ describe('@vaadin/hilla-react-crud', () => { } get name(): StringModel { - return this[_getPropertyModel]('name', (parent, key) => new StringModel(parent, key, false)); + return this[_getPropertyModel]('name', (parent, key) => new StringModel(parent, key, true)); } } const customService = createService(nullData); From 341c147d651d6d76957296e8e483570d44be8209 Mon Sep 17 00:00:00 2001 From: Krisjanis Seglins Date: Mon, 9 Sep 2024 15:55:35 +0300 Subject: [PATCH 5/6] Fix model creation for entity --- packages/ts/react-crud/test/autocrud.spec.tsx | 40 +++++-------------- .../test/test-models-and-services.ts | 30 +++++++++++++- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/packages/ts/react-crud/test/autocrud.spec.tsx b/packages/ts/react-crud/test/autocrud.spec.tsx index 39a9ed6dd1..decc6f5d91 100644 --- a/packages/ts/react-crud/test/autocrud.spec.tsx +++ b/packages/ts/react-crud/test/autocrud.spec.tsx @@ -3,13 +3,6 @@ import { expect, use } from '@esm-bundle/chai'; import { render, screen, waitForElementToBeRemoved, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { - _getPropertyModel, - makeObjectEmptyValueCreator, - NumberModel, - ObjectModel, - StringModel, -} from '@vaadin/hilla-lit-form'; import { TextField } from '@vaadin/react-components/TextField.js'; import chaiDom from 'chai-dom'; import sinon from 'sinon'; @@ -19,7 +12,14 @@ import ConfirmDialogController from './ConfirmDialogController.js'; import { CrudController } from './CrudController.js'; import FormController from './FormController'; import GridController from './GridController'; -import { createService, getItem, PersonModel, personService } from './test-models-and-services.js'; +import { + createService, + getItem, + PersonModel, + personService, + type UserData, + UserDataModel, +} from './test-models-and-services.js'; use(sinonChai); use(chaiDom); @@ -333,34 +333,14 @@ describe('@vaadin/hilla-react-crud', () => { }); it('opens the form in a dialog when row has undefined value', async () => { - type PData = { - id: number; - name?: number; - }; - const nullData: PData[] = [ + const nullData: UserData[] = [ { id: 1, name: undefined, }, ]; - class PDataModel extends ObjectModel { - static override createEmptyValue = makeObjectEmptyValueCreator(PDataModel); - get id(): NumberModel { - return this[_getPropertyModel]( - 'id', - (parent, key) => - new NumberModel(parent, key, true, { - meta: { annotations: [{ name: 'jakarta.persistence.Id' }], javaType: 'java.lang.Long' }, - }), - ); - } - - get name(): StringModel { - return this[_getPropertyModel]('name', (parent, key) => new StringModel(parent, key, true)); - } - } const customService = createService(nullData); - const result = render(); + const result = render(); const grid = await GridController.init(result, user); await grid.toggleRowSelected(0); diff --git a/packages/ts/react-crud/test/test-models-and-services.ts b/packages/ts/react-crud/test/test-models-and-services.ts index 3555f3bb5b..021574a0de 100644 --- a/packages/ts/react-crud/test/test-models-and-services.ts +++ b/packages/ts/react-crud/test/test-models-and-services.ts @@ -39,7 +39,7 @@ export interface Address { } export interface Department extends HasIdVersion { - name: string; + name?: string; } export interface Person extends HasIdVersion, Named { @@ -74,6 +74,10 @@ export interface ColumnRendererTestValues extends HasIdVersion { nested?: NestedTestValues; } +export interface UserData extends HasIdVersion { + name?: string; +} + class GenderModel extends EnumModel { static override createEmptyValue = makeEnumEmptyValueCreator(GenderModel); readonly [_enum] = Gender; @@ -349,6 +353,30 @@ export class ColumnRendererTestModel< } } +export class UserDataModel extends ObjectModel { + static override createEmptyValue = makeObjectEmptyValueCreator(UserDataModel); + + get id(): NumberModel { + return this[_getPropertyModel]( + 'id', + (parent, key) => + new NumberModel(parent, key, true, { meta: { annotations: [{ name: 'jakarta.persistence.Id' }] } }), + ); + } + + get version(): NumberModel { + return this[_getPropertyModel]( + 'version', + (parent, key) => + new NumberModel(parent, key, true, { meta: { annotations: [{ name: 'jakarta.persistence.Version' }] } }), + ); + } + + get name(): StringModel { + return this[_getPropertyModel]('name', (parent, key) => new StringModel(parent, key, true)); + } +} + type HasIdVersion = { id?: number; version?: number; From e149d9139c9441a4219d8327d114f52fd8b9a356 Mon Sep 17 00:00:00 2001 From: Kriss Seglins <101052651+krissvaa@users.noreply.github.com> Date: Tue, 10 Sep 2024 19:16:34 +0300 Subject: [PATCH 6/6] Update packages/ts/react-form/test/useForm.spec.tsx Co-authored-by: Soroosh Taefi --- packages/ts/react-form/test/useForm.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ts/react-form/test/useForm.spec.tsx b/packages/ts/react-form/test/useForm.spec.tsx index ab46fbcee3..09361f65e3 100644 --- a/packages/ts/react-form/test/useForm.spec.tsx +++ b/packages/ts/react-form/test/useForm.spec.tsx @@ -6,7 +6,7 @@ import chaiDom from 'chai-dom'; import { useEffect, useState } from 'react'; import sinon from 'sinon'; import sinonChai from 'sinon-chai'; -import { useForm as _useForm, useFormArrayPart, useFormPart } from '../src'; +import { useForm as _useForm, useFormArrayPart, useFormPart } from '../src/index.js'; import { type Contract, EntityModel,