Move form state into the global store

Worked on this at Drupalcon

Having some issues with cypress locally. The system module's entity types aren't being discovered when the system module's config is installed

This is now passing tests and ready for review

Looks great to me. Would be good to get @bnjmnm's eyes on this too.

EVERYTHING BELOW IS NOT OCCURRING WHEN I RUN E2E TESTS, SO I'M RELUCTANT TO EVEN MENTION IT But this is a big enough structural change that I'm going to document this in case it surfaces something legit. Perhaps something in the e2e test setup is addressing the underlying cause?

With this branch I see the following as soon as I load any article I visit in XB, new or existing. I also checked on a new Drupal install / different browsers to rule out autosave data messing with things. The screenshot is for the TextField component but I also run into it which other components.
Warning: `value` prop on `input` should not be null. Consider using an empty string to clear the component or `undefined` for uncontrolled components. Error Component Stack... (see screenshot → for the whole darn stack)

If I pull in a new SDC, the form is not populated with the default values

Unassigning myself for now, and I'll see if anyone is around to check this on their local.

I can reproduce #9 including on a completely fresh installation. Seems fundamental enough to require additional tests.

The problem is now fixed. I re-introduced maintaining a local state for the input values. This is necessary because we remove the value attribute from the input element in order to avoid getting warnings from React that a controlled component doesn't have event handlers. (See the relevant code in 0.x.) With this attribute removed, reading the value of text inputs on subsequent re-renders — of which we do have a couple — is not possible. Unless we have it saved in a local state.

I don't fully understand how this was not caught by the tests, but I do have a theory. The only thing I can think of is that the value attribute is there on the initial render, which I was able to confirm via debugging, but I wasn't able to confirm in a way that would actually paint it on the UI. The test code might have been able to get a hold of that value attribute before the component got re-rendered, and deemed the assertion true.

balintbrews → credited jessebaker → .

I ruled out my theory for the reason behind the test failures. Adding cy.wait() in those tests doesn't make them fail — it should surface the problem if it was a race condition with the component re-renders.

Then I remembered something @jessebaker said on Slack: maybe it's a dev vs. production build issue. That's it! 💫

I was able to confirm that testing the app after npm run build (and reverting my fix) doesn't present the issue! This is because there are no subsequent re-renders occurring for the InputBehaviors component in the production build. They happen in development mode thanks to our use of <StrictMode>, which helps us catch bugs by double rendering in development. That's exactly what happened here. The fix is still valid and necessary.

I don't see the need for updating our tests: the production build was not broken. Lower-level component tests could test this, but I feel like that would be out of scope for this issue.

Great debugging @balintbrews. I'm glad my hunch was at least along the right track and helped out some.

This is a really nice collaboration - nice work everyone.

@balintbrews in #11: wow! 😳 Quite the rabbit hole! And 👏🥳 for #13!!!

Idea: if we run the UI test suite with both dev and prod builds of the UI, we'd have been able to catch this automatically! Issue + MR: 📌 CI: new `cypress E2E debug` job Active .

Automatically closed - issue fixed for 2 weeks with no activity.