[Amoifr]

Fix #50

When a dependency changes, the UI re-renders its dependent field emptied. The submitted stale value was already handled for transformation failures: the error is cleared and moved to the hidden __dynamic_error field, so nothing is rendered but the form stays invalid.

Constraint violations were not: returning to a previous option submits the dependent field empty, and a NotBlank violation popped up on a field the user never got to fill (the error shown in the issue's video).

This applies the same treatment to violations, but only when a dependency was actually changed since the previous render:

• Each render stores the values of the dependencies in a new hidden, unmapped __dynamic_state field.
• On submit, the submitted dependency values are compared with that state. When one of them differs and the dependent field was submitted empty, its errors are cleared and the hidden error keeps the form invalid.
• Errors still render when no dependency changed since the previous render (e.g. submitting again right after a re-render, or submitting the initial value with an empty dependent field), when the state is missing, for disabled dependencies (browsers don't submit them) and for a multiple choice with nothing selected (browsers omit it).

The README's "Write custom JavaScript" section now mentions that __dynamic_state must be replaced along with the re-rendered fields.

Ships functional tests on a new fixture form carrying a NotBlank constraint (symfony/validator added to require-dev), walking the exact A / B / A flow from the issue's video, plus form-level tests for the edge cases above.

[bocharsky-bw]

Thanks for working on this with your suggestion! And sorry for the long reply. Fairly speaking, this is a complex one, and it seems it brings more problems, at least in the current implementation, see my comments below for details

[bocharsky-bw]

let's use strict comparison here

[Amoifr]

Done: the comparison is now a strict !== on the raw submitted values, and the loose != on model data went away with the rework below.

[bocharsky-bw]

This check is tied to the layout; maybe better to check the error message instead? E.g:
assertNotSee('This value should not be blank.')?

[Amoifr]

Good call, done. For the transformation failure step I gave the fixture field an explicit invalid_message: the default text differs between Symfony 5.4 ("This value is not valid.") and 6.0+ ("The selected choice is invalid."), so asserting it would pass for the wrong reason on lowest deps.

[bocharsky-bw]

It should be private

[Amoifr]

Done.

[bocharsky-bw]

Thanks for digging into this! I think there's a problem with how "changed" is detected here, and it seems a blocker.

This compares the submitted value with the form's initial data, not with what the user saw on the previous render. Once the dependency differs from its initial value, it's considered "changed" on every subsequent submit, including the final one where the user simply forgot to fill the dependent field. So the NotBlank error gets hidden, the form stays invalid, and the user has no idea why.

Reproduce with your fixture (this fails on the current branch):

public function testErrorIsRenderedOnSubmitAfterDependencyChangedInPreviousRender(): void
{
    $browser = $this->browser()
        ->visit('/constrained-form')
        ->selectFieldOption('Meal', 'Dinner')
        ->click('Submit Form') // re-render after the dependency changed: fine to hide
        ->assertNotSee('This value should not be blank.');

    // Meal is still "Dinner", the user submits without picking a food
    $browser->click('Submit Form')
        ->assertSee('Is Form Valid: no')
        ->assertSee('This value should not be blank.'); // fails: the error is hidden
}

It's even more visible in real apps: for a new entity, the dependency's initial data is usually null, so any selected value counts as "changed", and violations on dependent fields are effectively never rendered.

What we actually need to know is whether the dependency changed since the last render. The server doesn't remember that, but the form can carry it. A possible prototype:

• Add a hidden, unmapped field (e.g. __dynamic_state, or reuse __dynamic_error) and, in POST_SET_DATA, store the view values of the dependency fields in it (e.g. JSON).
• In the root form's PRE_SUBMIT, compare the raw submitted dependency values with that stored state. That gives you the real list of changed dependencies. Then overwrite the field with the current values via $event->setData(), so the next render carries the new state.
• If the state is missing (e.g. the field wasn't rendered), fall back to not hiding errors. Showing an extra error is much safer than silently blocking the submit.

With that, all existing tests plus both of yours should pass, and so does the case above. As a bonus, it compares raw strings instead of != on model data, which avoids comparing entities property by property.

Or let me know if you have a better idea. The key requirement is just that an empty dependent field must still show its error when the dependency did not change since the previous render. Wdyt?

[Amoifr]

You're right, and thanks a lot for the repro, that one would have been nasty in real apps. I went with your prototype: a hidden unmapped __dynamic_state field (kept separate from __dynamic_error so each field has one job) gets the dependencies' view values in POST_SET_DATA, and PRE_SUBMIT compares the raw submitted values with it, then writes the current values back so the next render carries them. A missing or unreadable state means nothing counts as changed, so a form rendered without form_rest() simply keeps today's behavior. Your test is in the PR as is, plus one for that fallback. (Side note: the two lowest deps jobs also fail on main.)

[bocharsky-bw]

Thanks a lot, this looks much better, and the fallback test is a nice touch! I found a few more edge cases, see the comment below.

Also 2 small things while you're at it:

• Could you add a note to the README's "C) Write custom JavaScript" section that the hidden __dynamic_state field must be re-rendered/replaced too? Otherwise it keeps the state of the first render and the errors get hidden again.
• The PR description still says "differs from its initial data", could you rephrase it according to the new approach?

[bocharsky-bw]

I played with a few more edge cases and found two where the original problem comes back: the error is hidden on every submit because a field the browser doesn't send is treated as "changed":

1. Browsers don't submit disabled fields, so $data[$dependency] is missing and becomes '', while the rendered state holds the real value (e.g. "b"). E.g. an edit form where "country" is disabled: the NotBlank error on the dependent "state" field is never shown.
2. Problem with multiple choice with nothing selected (multiple: true, expanded or not) where the rendered state is [], but the browser omits the field entirely, so it becomes '' and [] !== ''.

To reproduce (the dependent notes field has NotBlank; both assertions fail on the current branch):

// disabled dependency, browser does not send "dep"
$form->submit(['notes' => '', '__dynamic_state' => '{"dep":"b"}']);
$this->assertCount(1, $form->get('notes')->getErrors());

// multiple choice with nothing selected, browser does not send "dep"
$form->submit(['notes' => '', '__dynamic_state' => '{"dep":[]}']);
$this->assertCount(1, $form->get('notes')->getErrors());

We probably should skip disabled dependencies in detectChangedDependencies() (Symfony ignores their submitted data anyway), e.g. if ($form->get($dependency)->isDisabled()) { continue; }. Or in normalizeDependencyValue(), normalize an empty array to '' as well, like null.

[Amoifr]

Great catches, thanks! Both are fixed with your two tests (they failed before the change):

• disabled dependencies are skipped in detectChangedDependencies(), and their rendered value is carried over to the next state;
• an empty array is normalized to '' like null, on both sides of the comparison, so a state rendered as [] also matches an omitted field.

I also added the README note about __dynamic_state in the "Write custom JavaScript" section and rewrote the PR description. Hopefully the edge case collection is complete now, but I won't bet my keyboard on it. 😄

[bocharsky-bw]

Could you please pull changes from main? The lowest jobs should be fixed now on main.

[bocharsky-bw]

This way we suppose that the form will render exactly like this. It would be more robust to let the form render it without guessing. I think this should help:

$renderedState = $this->createFormWithDependency()
        ->createView()['__dynamic_state']->vars['value'];

If it works - we will just take this $renderedState and pass to the submit() as is. If it ever changed - we will notice it with tests. Wdyt?

[Amoifr]

Good idea, done: both tests now render the form first and submit the __dynamic_state value from its view, so they no longer assume what the state looks like. They still fail without the fix.

I also merged main (one conflict in composer.json, symfony/validator now follows the new ^5.4|^6.4|^7.0|^8.0 constraints). The suite passes locally with --prefer-lowest too, on Symfony 5.4.0. 🙂

[bocharsky-bw]

Thanks for working on it and improving the code on my request 👍🏼