Skip to content

Conversation

@norkunas
Copy link
Contributor

@norkunas norkunas commented Jan 21, 2022

Q A
Bug fix? yes
New feature? no
Tickets N/A
License MIT

When creating a form builder with LiveComponent I've found out that proper values are not populated when form changes.
For example:

  1. Form has many questions;
  2. Question have a type field (text/choice/date/datetime, etc) and then dynamically added options field based on it's type;
  3. Date/datetime has min/max options;
  4. I change type to datetime:
    1. Component renders min/max option inputs with name like form_builder[questions][0][options][minDateTime][date][year];
    2. It gets two levels deeper with [date][year] because that how Symfony renders datetime by default;
    3. Trying to change value of an option - get UI LiveController error that it's not exposed;
    4. And that's because formValues were already extracted without that deep nested data.

Not sure about the reason why it was done this way previously. My guess would be for perf? Maybe this could live as some trait option by which getFormValues would decide to always re-extract or take from cache?

@norkunas norkunas force-pushed the form-values-nocache branch from a92fe82 to dde4a65 Compare January 21, 2022 05:52
@Lustmored
Copy link
Contributor

I was just attacking same problem, but with different angle in #221. I am afraid your solution could lead to loss of filled form values on each render (frontend would still show them, but they could be wrong in the backend). Do you have a test case where we can see the problem described here? And could you check if it still happens on #221 branch?

@norkunas
Copy link
Contributor Author

I don't see any loss of data in my app :)

@norkunas
Copy link
Contributor Author

I can live currently without this as I have a workaround, so if you think your solution is better than I can close

@Lustmored
Copy link
Contributor

I am not a person in power here, just trying to understand your problem better to see if it collides with my work in other PR 👍

if (null === $this->formValues) {
$this->formValues = $this->extractFormValues($this->getForm());
}
$this->formValues = $this->extractFormValues($this->getForm());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you were able to put together a failing test, that would be awesome. As you know, this change is also in #221 (with a slight difference). But, at the moment, even in that PR, there are no tests covering the problem (i.e. if I revert that change in #221, its tests still pass). So, a test to show off the problem would be 💯

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lustmored PR covers more cases than mine, so probably It'd be better to leave to put that test in it's PR, so closing this one :)

@norkunas norkunas closed this Jan 24, 2022
@norkunas norkunas deleted the form-values-nocache branch January 24, 2022 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants