Skip to content
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

[LiveComponent] Embeded stimulus controller in a live form not working on re-render #489

Closed
tdumalin opened this issue Oct 4, 2022 · 5 comments

Comments

@tdumalin
Copy link

tdumalin commented Oct 4, 2022

Hi,

I want to use symfony live component to render a form and use some other controller embeded in the form.
Some example may be to use a datepicker. I used this one: https://www.npmjs.com/package/stimulus-datepicker
I've managed to use the controller in the form but when i change an input the component re-render is triggered and the stimulus controller doesn't work anymore.

A solution may be to use data-live-ignore but in my project i use this picker in a LiveCollectionType entry so i need the controller to be connected after a re-render

I've created a mini project to show the problem:
https://github.com/tdumalin/bug-live-component.git

Installation (assumes that php, symfony cli,composer and yarn are installed):

git clone https://github.com/tdumalin/bug-live-component.git
composer update
yarn install
yarn build
symfony serve -d

http://127.0.0.1:8000/

When you click on "Toggle" the picker shows but if you change the select value and re-click on "Toggle" stimulus controller is not triggered anymore

@weaverryan
Copy link
Member

Hi @tdumalin!

Thanks for the reproducer! That was super helpful! This is definitely an area we want to make smoother, even if it's just via documentation.

In this case, when the Stimulus controller's connect() is called, it makes a number of changes to the HTML around your input: https://github.com/airblade/stimulus-datepicker/blob/b672a0da67f28520cd1fc7a3375f44632122a595/src/datepicker.js#L38-L41

When the component re-renders, we have 2 options:

A) We make sure to NOT overwrite these changes - i.e. we add data-live-ignore
B) We overwrite the field, but then reinitialize the Stimulus controller.

As you guessed, (A) is definitely the first thing to try. I added it to the <div class="input-group mb-3" that's around the entire field and it worked fine. But you mentioned a LiveCollectionType? Have you tried adding data-live-ignore to this element in your real situation? What bad behavior does it cause?

For (B), what we can do is force LiveComponents to TOTALLY re-render the "date" field instead of just "modifying the things inside that changed". This will trigger the datepicker Stimulus controller to be removed and a totally new one to be added. To do this, you would give that same <div class="input-group mb-3" element a data-live-id attribute... but this attribute would need to CHANGE (to some new, unique value) each time you wanted this field to totally re-render. Currently, I'm not sure this is really an option, because you would need to make sure that the data-live-id ALWAYS changes (like, set it to a random value) so that it is always re-rendered.

Another interesting approach might be to combine the two - e.g. <div class="input-group" data-live-ignore data-live-id="foo" where you use data-live-ignore to fix the problem (solution A). But then, if, for some reason, you ever needed the field to totally re-render (maybe the value of this field was just changed on the server), you could change the data-live-id to some other value. However, currently, this doesn't work, as data-live-ignore tells the system to "never remove this element on re-render". But if this solution sounds useful for your situation, we can explore how to make it possible.

Cheers!

@tdumalin
Copy link
Author

tdumalin commented Oct 5, 2022

Hi @weaverryan

Thanks for the help, i tried to apply your advice, with data-live-id containing an unique value, the datepicker can now be used after a re-render but the value of the field is lost on each re-render triggered by another form field.
You can check the behavior by pulling the demo project: https://github.com/tdumalin/bug-live-component.git

Some changes:

  • use https://www.npmjs.com/package/js-datepicker and create a custom controller in /assets/controllers/datepicker_controller.js
  • add a LiveCollection to check the behavior
  • set fields in templates/form/fields.html.twig to re-use same logic on each DateType
  • use a FormExtension to autoset DateType default options

@weaverryan
Copy link
Member

Hi @tdumalin!

Ok, I see your random data-live-id. I think that's a hacky thing to force people to do (to trigger the re-render), but it's working. Perhaps we should introduce something like data-ignore="false" which would mean "ALWAYS re-render"? Or maybe some other attribute would be more clear to mean "always re-render this element on update"?

About the problem, it's a classic problem with external JavaScript: https://symfony.com/bundles/ux-live-component/current/index.html#updating-a-field-via-custom-javascript

With a normal input field, when you change it, it triggers a change event. Live components listeners for this event to update its model internally. But in this case, the js-datepicker is changing the value of the input, but is NOT triggering the change event. So live components never has any idea it changed. When, when you change a different field and trigger a re-render, the updated value for the date is missing. Ideally, the date library would just do that automatically. But since it doesn't, you would need to write some custom JavaScript (which starts to defeat the value of live components) to listen to the onSelect internal event - https://www.npmjs.com/package/js-datepicker#onselect - of that library and trigger the change event like we show in the docs.

It strikes me that, as this will be common, we'll want to offer SOME better way to work around this limitation in libraries.

@tdumalin
Copy link
Author

tdumalin commented Oct 5, 2022

@weaverryan

Exactly what i need to know ! thank you for all your explanation, the way live component works is way more clear now.
For info i've pushed a working version of datepicker stimulus controller on my project repository if needed.

Use the data-live-id="{{ random() }}" is some kind of weird, i would say that you should add an attribute called "data-rerender-behavior" with string value like: always|never|onChange
That would be more "readable" and easy to understand

Thanks again for the help !

@weaverryan
Copy link
Member

Thanks for the suggestion - I opened an issue at #490 - but I wasn't sure what you had in mind for the onChange value.

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

No branches or pull requests

2 participants