Form API #states property/states should use .once() to apply its rules (Can cause failures with BigPipe and possibly other situations)

Thank you for creating the issue and providing a patch.
Bugs should be fixed in the latest dev version of Drupal and then backported to older versions like 9.5.x when applicable.
Since D10 doesn't use es6.js files anymore the patch needs to be different for D10.

Also we should provide one of this scenarios as a js test to be able ensure the behaviour stays consistent in the future

I can confirm this issue atleast in 9.5 and 9.2, the patch at #3 resolves it for me, atleast in my situation BigPipe is not used when anonymous/not-logged-in, but on logged in, bigpipe is activated and then I get missing form states, btw @evaldas-užkuras thanks!!

There is this one which is sort of related https://www.drupal.org/project/drupal/issues/3337995 but was closed

I'm a bit surprised that #states wasn't using once(), it's even declared as dependency to the drupal.states library.
This is definitely a bug which affects BigPipe and AJAX use cases. The patch looks correct (didn't test it though).

Just throwing this out there, maybe `states` doesnt bind with the right 'context' ? https://www.drupal.org/project/drupal/issues/3337995#comment-15060748

I already have trouble convincing front-end developers to use the context parameter in our custom modules (most of them just ignore it since don't understand its logic). Now I have to convince them to use jQuery once. When the rest of the javascript world is moving away from jQuery we are forcing people to use it. It doesn't make any sense.

Edit: I just realized that Drupal has its own version of once that is compatible with vanilla javascript but I still think we need a javascript API easier to understand for frontend developers that aren't specialized in Drupal.

Did not test.

But was previously tagged for tests which still need to happen. May be worth posting in #testing channel for suggestions where to add.

This was a tricky one to debug, but doing so eventually led to me here today. Here's an example form that is affected by this bug which I think can be used as basis for a regression test of the fix:

    $form['select'] = [
      '#type' => 'select',
      '#title' => 'Select',
      '#options' => [
        'one' => 'one',
        'two' => 'two',
        'three' => 'three',
      ],
      '#default_value' => 'one',
    ];
    $form['checkbox'] = [
      '#type' => 'checkbox',
      '#title' => 'Checkbox One',
    ];
    $form['textfield'] = [
      '#type' => 'textfield',
      '#title' => 'Textfield',
      '#states' => [
        'visible' => [
          ':input[name="select"]' => ['value' => 'one'],
          ':input[name="checkbox"]' => ['checked' => TRUE],
        ],
      ]
    ];

In this example, if the select box has the first option selected AND the checkbox is checked, the textfield should be shown. However, on page load w/ Big Pipe enabled, checking the checkbox (which meets conditions) does NOT show the textfield. You have to uncheck and then recheck it before it finally works.

Big Pipe causes two more invocations of Drupal behaviors, resulting in a total of 3 invocations with the form items in the context. That's enough to cause issues with the logic with the 3 duplicate states behaviors defined.

I just closed #3383972: States JavaScript should use the once library, causes problems with Big Pipe as a duplicate of this issue. There is some information on how to reproduce with the contributed Webform module in the IS of that issue and MR with a similar but slightly different approach to fixing this.

Enabled big_pipe module on existing JavascriptStatesTest. Test-only job should show it failing.

Well GitLab CI made a liar out of me. For some reason it decided to skip the test instead of running it and letting it fail. Attaching a patch to see what Drupal CI thinks.

FWIW this is the result when running the test-only changes locally:

$phpunit --filter=JavascriptStatesTest
PHPUnit 9.6.15 by Sebastian Bergmann and contributors.

Testing 
F                                                                   1 / 1 (100%)

Time: 00:28.863, Memory: 357.00 MB

There was 1 failure:

1) Drupal\FunctionalJavascriptTests\Core\Form\JavascriptStatesTest::testJavascriptStates
Failed asserting that true is false.

/data/app/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121
/data/app/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/data/app/core/tests/Drupal/FunctionalJavascriptTests/Core/Form/JavascriptStatesTest.php:462
/data/app/core/tests/Drupal/FunctionalJavascriptTests/Core/Form/JavascriptStatesTest.php:70
/data/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

FAILURES!
Tests: 1, Assertions: 205, Failures: 1.

Hiding #20 so bot doesn't pick it up.

Going to post in gitlab channel but I see this test-only job failed correctly https://git.drupalcode.org/issue/drupal-3347144/-/jobs/471592

But change seems good, had to look up the param changes but looks good.

I'm triaging RTBC issues. I read the IS and the comments. I didn't find any unanswered questions or other work to do.

Leaving at RTBC.

Committed/pushed to 11.x and cherry-picked to 10.2.x, thanks!

Related regression ticket is here: https://www.drupal.org/project/drupal/issues/3416398

The regression in #3416398: Regression on entity reference field states seems like it might be worse than the bug with big pipe that this fixed - I'm considering rolling this back in 11.x and 10.2.x, so we can try again.

Hmm, tricky. So it seems the problem is that states sometimes needs to be applied again because the dependents may have changed, right?

In that case, I guess we need a way for states behavior to replace the existing state that was there before so it doesn't create duplicate state definitions.

Discussed this briefly with nod_ in slack and we both think reverting is probably the best approach here - more sites are still on 10.1.x than 10.2.x, so reverting the bug fixed here won't be a regression for them, but the regression introduced with be a new bug when those sites update. Also doesn't seem like there's an easy fix for this in either direction, we probably can't use once, and might need to make the states javascript re-entrant instead, no idea what that means in practice.

Rerolling and marking as needs tests to cover the bug found in #3416398: Regression on entity reference field states

This issue is a duplicate of #3386191: #states not working correctly when built from a logical combination of multliple fields

It looks like the fix in #3386191: #states not working correctly when built from a logical combination of multliple fields was the exact same fix that had already been committed and reverted in this issue. So I'm wondering if we are still concerned about the issues in #34? I'm guessing not since @node_ was the one who committed it. Perhaps credit should be transferred from this issue as well. Setting to PMNMI to see if #34 still needs addressing.

let's take that on in #3468860: JS #states behavior does not have a detach method,

The tests are being done in #3468860: JS #states behavior does not have a detach method

a proper fix is in the works for core, could you test #3468860: JS #states behavior does not have a detach method. It should fix this but introduces another change in that #states about a missing element are now applied, they used to be ignored.