Skip to content

Always re-extract formValues in LiveComponent #234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

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