feat: form binder object models support #3816

platosha · 2025-07-31T10:30:49Z

…ted types

… much hassle

…iggering TS2589

codecov · 2025-10-06T07:28:01Z

Codecov Report

❌ Patch coverage is 97.05882% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.69%. Comparing base (75a2378) to head (d500bf6).

Files with missing lines	Patch %	Lines
packages/ts/lit-form/src/stringConverters.ts	82.35%	9 Missing ⚠️
packages/ts/lit-form/src/BinderNode.ts	95.65%	4 Missing and 2 partials ⚠️
packages/ts/models/src/m.ts	98.41%	2 Missing ⚠️
packages/ts/lit-form/src/Validation.ts	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3816      +/-   ##
==========================================
+ Coverage   86.27%   86.69%   +0.41%     
==========================================
  Files         123      127       +4     
  Lines        8474     8764     +290     
  Branches     1301     1356      +55     
==========================================
+ Hits         7311     7598     +287     
- Misses       1140     1143       +3     
  Partials       23       23

Flag	Coverage Δ
unittests	`86.69% <97.05%> (+0.41%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sonarqubecloud · 2025-10-07T14:00:56Z

Quality Gate failed

Failed conditions
3 Security Hotspots
54.8% Duplication on New Code (required ≤ 3%)
D Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Lodin · 2025-10-07T13:59:50Z

packages/java/tests/csrf-context/package.json

    "@vaadin/hilla-lit-form": "file:../../../ts/lit-form",
-    "@vaadin/icon": "25.0.0-alpha21",
-    "@vaadin/icons": "25.0.0-alpha21",
+    "@vaadin/icon": "25.0.0-alpha20",


Maybe, this is wrong? 🤔

Lodin · 2025-10-07T14:02:01Z

packages/ts/lit-form/src/Binder.ts

-  constructor(context: Element, Model: DetachedModelConstructor<M>, config?: BinderConfiguration<Value<M>>) {
+  constructor(
+    context: Element,
+    modelClass: DetachedModelConstructor<M & AbstractModel> | (M & Model),


Shouldn't it also be ProvisionalModel?

Lodin · 2025-10-07T14:05:29Z

packages/ts/lit-form/src/BinderNode.ts

+function getModelValidators<M extends ProvisionalModel>(model: M): ReadonlyArray<Validator<Value<M>>> {
+  if (model instanceof AbstractModel) {
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+    return model[_validators];
+  }
+
+  const validators: Array<Validator<Value<M>>> =
+    model instanceof NumberModel ? [new Validators.IsNumber(model[$optional]) as Validator<Value<M>>] : [];
+
+  for (const constraint of model[$constraints]) {
+    const validator = getConstraintValidator<Value<M>>(constraint);
+    validators.push(validator);
+  }
+
+  return validators;
+}
+
+function createEmptyValue<M extends ProvisionalModel>(model: M) {
+  if (model instanceof AbstractModel) {
+    return model.constructor.createEmptyValue();
+  }
+
+  return model[$defaultValue];
+}


I guess, it'd need some docs to describe that there are some checks for transition period. Or, maybe, I'd put it in another file with all the proxies.

Lodin · 2025-10-07T14:07:02Z

packages/ts/lit-form/src/BinderNode.ts

-    while (model[_parent] instanceof AbstractModel) {
-      name = `${String(model[_key])}${name ? `.${name}` : ''}`;
-      model = model[_parent];
+    while (model[$owner] instanceof AbstractModel || model[$owner] === Model || model[$owner] instanceof Model) {


Suggested change

while (model[$owner] instanceof AbstractModel || model[$owner] === Model || model[$owner] instanceof Model) {

while (model[$owner] instanceof AbstractModel || model[$owner] instanceof Model) {

You don't have to separately check if it is a Model. instanceof should do it for you.

Lodin · 2025-10-07T14:07:56Z

packages/ts/lit-form/src/BinderNode.ts

    this.initializeValue();

-    const key = this.model[_key];
+    const key = this.model[$key];


I was confused seeing a $key here. Probably, it requires a small description in a doc comment

Lodin · 2025-10-07T14:50:01Z

packages/ts/models/src/Model.ts

+export const $from = Symbol('from');
+export const $to = Symbol('to');
+
+export type ModelConverter<FM extends Model = Model, TM extends Model = Model> = {


I'd also added an explicit reference to that StackOverflow question.

Lodin · 2025-10-07T14:51:27Z

packages/ts/models/src/models.ts

+/**
+ * The model of a `Record<string, V>` data, which is a special case
+ * of `ObjectModel<Record<string, V>>` that is used to represent an arbitrary
+ * object with string keys, such as a Java `Map<String, Object>`.


Why not the MapModel then? 🤔

Lodin · 2025-10-07T14:52:28Z

packages/ts/models/src/m.ts

+ *
+ * @param model - The model to get the value from.
+ */
+export function getValue<T>(this: void, model: Model<T>): T {


Why not just a simple value?

Lodin · 2025-10-07T14:53:46Z

packages/ts/react-form/src/index.ts

-export function useForm<M extends AbstractModel>(
-  Model: DetachedModelConstructor<M>,
+export function useForm<M extends ProvisionalModel>(
+  modelClass: DetachedModelConstructor<M & AbstractModel> | (M & Model),


ProvisionModel?

Lodin · 2025-10-07T14:54:15Z

packages/ts/react-form/test/useForm.spec.tsx

 describe('@vaadin/hilla-react-form', () => {
  type UseFormSpy = sinon.SinonSpy<Parameters<typeof _useForm>, ReturnType<typeof _useForm>>;
-  const useForm = sinon.spy(_useForm) as typeof _useForm;
+  const useForm = sinon.spy(_useForm) as unknown as typeof _useForm;


Is it possible to remove cast to unknown? It looks kinda broken

feat(lit-form): preliminary object models support

bd8fe14

Fixes #3436

platosha added the target/24.9 label Jul 31, 2025

platosha added 28 commits August 4, 2025 16:56

Merge branch 'main' into feat/model-object-binder

19e9069

feat(models): constraints support

cbe7a03

refactor(models): simplify constraint definitions

3e9b929

fix(models): constraint test fixes

aef40b0

feat(form): support object model constraints

5ff5c36

fix(models): allow any key

82a1db2

fix(models): TypeScript usage issues of optional, array and constrain…

11cb276

…ted types

refactor(models): move constraints to separate directory

00c466c

refactor(models): move constraints back to src directory, dirs are to…

969d8e3

… much hassle

feat(models): missing features and fixes for binder

2bf1f19

text(lit-form): use object models in tests

da244d6

feat(models): make optional key have undefined default value

6fadcde

fix(lit-form): make basic value init work with object model

4565100

fix(lit-form): basic array and object operations with object models

f7f3fb9

test(lit-form): forgotten constraint in test fixture

a101edd

fix(lit-form): correct binder node name

e919ca0

test(lit-form): correct model name

28d9eb4

fix(models): improve constrant definitions for binder

2935b33

fix(lit-form): support object models in field directive

6c5f757

fix(lit-form): field support for string object model

88e34d4

fix(lit-form): combo box field support with array object models

6b54ca7

feat(models): enable optional self-referencing arrays

e6401f3

feat(models): optional array models

cef1fdc

feat(models): nested self-referencing models

29a6e58

fix(models): keep original default value in optional models

9bc6f8c

fix(models): make default value simple objects

d3e519a

test(lit-form): correct fixture expectations

88e803c

perf(models): cleanup & replace aliases with mapped types to avoid tr…

663c4a8

…iggering TS2589

platosha added 11 commits October 2, 2025 12:32

fix(models): make number model carry NaN (empty value) by default

1d71b2b

feat(react-form): support object models

4fddfab

feat(lit-form): expose stringConverter utility for react-form

ebe0b45

fix!(models): simplify object models more

e18ea66

fix(models): deep self-referencing models

b6244f7

fix(models): test fixes

ccf055d

test(react-form): handle unhandled rejection in tests

66f2fb7

test(react-form): bring old tests back for now

21f3ee3

Merge branch 'main' into feat/model-object-binder

acf078f

fix(models): circular dependency

bcdcf85

chore: update package-lock.json

e18ab24

platosha changed the title ~~feat(lit-form): preliminary object models support~~ feat(lit-form, react-form): object models support Oct 6, 2025

platosha changed the title ~~feat(lit-form, react-form): object models support~~ feat: object models support Oct 6, 2025

platosha changed the title ~~feat: object models support~~ feat: form binder object models support Oct 6, 2025

platosha removed the target/24.9 label Oct 6, 2025

platosha marked this pull request as ready for review October 6, 2025 07:23

platosha and others added 6 commits October 6, 2025 15:45

fix(models): simplify self references implementation

4ca417f

fix(models): make output prettier for simple types

89732d2

Merge branch 'main' into feat/model-object-binder

77531df

chore: update package-lock.json

ba89908

fix: TypeScript errors

54dadb4

Merge branch 'main' into feat/model-object-binder

d500bf6

Lodin suggested changes Oct 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: form binder object models support #3816

feat: form binder object models support #3816

Uh oh!

platosha commented Jul 31, 2025

Uh oh!

codecov bot commented Oct 6, 2025 •

edited

Loading

Uh oh!

sonarqubecloud bot commented Oct 7, 2025

Uh oh!

Lodin Oct 7, 2025

Uh oh!

Lodin Oct 7, 2025

Uh oh!

Lodin Oct 7, 2025

Uh oh!

Lodin Oct 7, 2025

Uh oh!

Lodin Oct 7, 2025

Uh oh!

Lodin Oct 7, 2025

Uh oh!

Lodin Oct 7, 2025

Uh oh!

Lodin Oct 7, 2025

Uh oh!

Lodin Oct 7, 2025

Uh oh!

Lodin Oct 7, 2025

Uh oh!

Uh oh!

	while (model[$owner] instanceof AbstractModel \|\| model[$owner] === Model \|\| model[$owner] instanceof Model) {
	while (model[$owner] instanceof AbstractModel \|\| model[$owner] instanceof Model) {

feat: form binder object models support #3816

Are you sure you want to change the base?

feat: form binder object models support #3816

Uh oh!

Conversation

platosha commented Jul 31, 2025

Uh oh!

codecov bot commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sonarqubecloud bot commented Oct 7, 2025

Quality Gate failed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Oct 6, 2025 •

edited

Loading