[LiveComponent] Meta issue for todos, changes & bugs

[weaverryan]

Hi!

Earlier, the TwigComponent and LiveComponent packages were introduced. Since the community is just getting its first look at these, and because there are known TODOs and ideas, I though it would be best to start with a single meta issue to track things :).

Some of these reference the "Demo" - found here: https://github.com/weaverryan/live-demo

Features

Feature A) Add support for @Security annotation/attribute. Also for @Cache annotation. #116

Feature B) Action arguments - e.g. data-action-name="delete(id=5)". The frontend syntax is already supported. We need to add a way to "allow" certain action arguments to be "passable" from the frontend. #218

Feature C) CSS Transition support for removed/added elements

Feature D) Ability to add cache headers in addition to the @Cache annotation #116

Feature E) Allow loading behavior to be specific to a model or action -e.g. data-loading="action(save)|show" to "show" this element only when the "save" action is loading. #470

Feature F) Ability to control the "route" for a component, instead of using the built-in /_components/{componentName}/{actionName}. And the same for specific actions of a component. WONTFIX unless requested with a good reason.

Feature G) Audit and add more JavaScript events that are dispatched.

Feature H) Testing tools: tools to help you create a component with some data, call actions on it, and assert final things on the rendered HTML.

Feature I) Allow controlling the debounce time on a component and model level.

Feature J) A system to allow for computed properties (e.g. a method that is called only once, then its value is returned every time after)

Feature K) A make:component command

Feature L) Look again at "child" components and how they communicate back and forth

Changes

Change A) Investigate making the initial render a sub-request. This would make initial render more consistent with a re-render, but we need to check the performance penalty. Will not do for now - can revisit later.

Change B) getComponentName() -> getName()

Change C) Investigate removing ComponentInterface and LiveComponentInterface in favor of annotations/attributes. #106

Change D) Consider forcing the template name to be configured (e.g. a template="" option on the annotation/attribute from above #106

Change E) Normalize naming of hooks (Before / Pre/Post) #281

Change F) Change annotation's constructor - #101 (comment)

Change G) Consider eliminating the this. prefix in the template... which may not actually be possible :). UPDATE: this is partially done by making public vars available. But Kevin and I also talked about adding a TemplateVariable attribute to allow you to expose non-public properties, or even methods.

Bugs

Bug A) Proper JavaScript packaging & IE11 support: #101 (comment) #101 (comment) #111

Bug B) Add missing PHP 8 attributes #106

Bug C) Child components don't update their parent component's model - seeable in the /admin forms of the demo https://github.com/weaverryan/live-demo - edit the textarea field, the submit via the "action" button (the new value will not be reflected) #113

Bug D) From the demo - https://github.com/weaverryan/live-demo - if you hit hide/show on the notifications as polling finishes, it's wonky. #278

Bug E) From the demo - https://github.com/weaverryan/live-demo - If you submit a new notification, the previous "message" remains in the input. But if you hit submit again, it submits as an empty string. #250

Bug F) Some way to force a child component to re-render when a parent re-renders. This can be seen if you make a child component - similar to EditPostNoFormComponent on the demo - have a "Deferred" update. Then click a button to trigger an "action". The content field (the textarea) would have an error. But it would not cause the child component to re-render, so the error would not be shown. #113

Bug G) Gracefully handle unexpected errors on the frontend & help the developer debug these. Currently, unexpected errors cause a Javascript error. Also, if you "forget" to set a LiveProp to writable: true and then modify it, this currently results in an "invalid checksum". Better error would be "Did you forget to set writable: true?". #467 and #466

Bug H) Add a way to mark an element as "permanent" so that it won't be replaced on render. This is useful if some other JavaScript has manipulated your element and added something.

Bug I) Check file uploads with actions #289

Bug J) Allow different "display" types with data-loading. Currently, when we "show" an element, we add display: inline-block (to override the display: none in CSS). That should be configurable on a per-element basis.

Bug K) Don't require a checksum on the URL if there are no read-only LiveProp. Or, possibly turning the checksum off for a component would be allowed (I'm thinking of writing your own JavaScript that, as the user types, you make a request to /components/users?email=ryan.

Bug L) What should happen if a deferred model change happens during an Ajax call? We can't simply use the new data from the new Ajax call, as that would replace the deferred model update. This might be a situation where a race condition must be allowed (probably using the new data from the Ajax call as the "source") and controlled by the user (e.g. if it's important, the user would disable a field during loading to avoid it changing).

Documentation

Docs A) Finish LiveComponent README ("hook" system - especially security is needed, also might need to change the "int" type on min and max to avoid problems with super bad values and document why).

Docs B) Some typos from #101 (review) #103

Docs C) Remove relative README links #101 (comment) #107

Docs D) Play with & document rendering problems with lists... and the use of the id attribute to fix. See the description next to getNodeKey() on morphdom: https://github.com/patrick-steele-idem/morphdom#morphdomfromnode-tonode-options--node

Docs E) Document limitations of the CSS that hides data-loading="show" elements, and the workaround. Specifically, if you have any exotic loading mechanism - e.g. data-loading="show addClass(foo)", the CSS would not automatically hide this: https://github.com/symfony/ux/blob/main/src/LiveComponent/assets/styles/live.css#L1 - and so a display: none needs to be manually added.

Docs F) Document using a DTO in a LiveProp / the dehydration system.

Docs G) Document the exposed={} option

Docs H) Add missing Flex recipe & also document "routing" import needed if you're not using Flex. #109

Docs I) Make sure "how to add security" is properly documented

Docs J) Mention that the property accessor is used, so properties can be private with a setter.

Minor

Minor A) See if LiveComponents can support php 7.4, or not. (won't do)

Minor B) Add code to CI to verify that all dist files are built.

Minor C) Consider using JSON.stringify in http_data_helper.js

Minor D) Remove/complete TODO's in LiveComponentHydrator and ComponentRenderer

Minor E) Move "private" methods & code out of live_controller.js

Minor F) Throw an error if args are passed to data-loading with hide/show - e.g. data-loading="hide(me)"

Future Ideas

Future A) A web debug toolbar integration

Future B) A debug:component command

Future C) Mercure support

Future D) Polling enhancements: (1) poll less often when the browser tab is not active and (2) don't start polling until visible

Future E) Add support for different keyup/keydown keys. For example: data-action="keyup->live#action" and data-action-name="key(enter).save key(p).someoneTypedP".

Future F) Add support to add styling for a "dirty" field (a field that has been updated, but a re-render hasn't happened yet, because updateDefer was used) - e.g. data-dirty="addClass(border-red-500)"

Future G) Potentially add support for "lazy" components (they don't load until they are visible) or at least document how one could use a lazy Turbo Frame nicely with a component URL. For example, on page load, a component has a "loading" animation. Then, it loads via Ajax and the area is updated.

Future H) Add support for dehydrating an entity object using something other than the primary key? e.g. UUID

[kbond]

F) [CHANGE] getComponentName() -> getName()

I think it's likely a user could have a "name" property.

[weaverryan]

I think it's likely a user could have a "name" property.

Good point. So that's contingent on:

G) [CHANGE] Investigate removing ComponentInterface and LiveComponentInterface in favor of annotations/attributes.

[jdreesen]

Docs 2) Some typos from #101 (review)

Here's a PR with the fixes: #103

[jordisala1991]

The idea for other packages here is to integrate with the new Twig component? (Like the chart.js)

[weaverryan]

The idea for other packages here is to integrate with the new Twig component? (Like the chart.js)

Let me make sure I understand your question first. I think you are asking:

Will the other UX libraries - like chart.js - be refactored to use the TwigComponents library?

If so, probably not :). I DO think there is some opportunity to build some UX libraries and leverage TwigComponent, especially if doing so gives users more flexibility. But many UX components (and chart.js is a great example) are SO simple that they wouldn't benefit from using TwigComponent (ultimately, in HTML, the chart.js library outputs an empty div with a data-controller on it and the chart data).

However, there is nothing preventing someone from putting chart.js inside of their own TwigComponent :). You could do this to (A) make it easier for you to build charts on your site, providing different input, and generating the Chart object inside your component class or (B) to make your chart part of a "live" component, which could re-render itself if the user changes some settings on it.

[dbrekelmans]

Hi, I'm looking into

Change C) Investigate removing ComponentInterface and LiveComponentInterface in favor of annotations/attributes.

Currently looking at TwigComponent first.

I think we could support three different options of configuring the component name (in this order of priority):

1. Pass the name in the tag (->tag('twig.component', ['name' => 'component_a'])
2. Add a PHP8 attribute to the class (#[Component('component_a')])
3. Add a doctrine annotation to the class (@Component(name="component_a"))

With the first one, the tag is set manually by the user. The second one we can autoconfigure using $container->registerAttributeForAutoconfiguration. The third one is trickier, since I haven't found a standardized way in the dependency injection component to autoconfigure a class based on the doctrine annotation. Perhaps we choose to not support autoconfiguration based on doctrine annotations? We can still check for the doctrine annotation in the compiler pass when registering the component by its name, so it will still work if the user didn't manually specify a name in the tag (but the service would have to be tagged manually by the user).

Looks like we can completely get rid of the ComponentInterface this way (and as a result get rid of the getComponentName() method, and avoid any conflicts with user defined properties on the component).

What do you think about this approach?

[kbond]

@dbrekelmans, minus the annotations, that's exactly what we've been discussing in #106.

I only just learned about registerAttributeForAutoconfiguration so I'm going to adapt that PR to use.

[kbond]

BTW, I think the consensus is to drop annotations entirely and make the new packages PHP8+.