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] Multiple URL query params in one LiveProp DTO #2142

Open
Nayte91 opened this issue Sep 9, 2024 · 17 comments
Open

[LiveComponent] Multiple URL query params in one LiveProp DTO #2142

Nayte91 opened this issue Sep 9, 2024 · 17 comments
Labels
LiveComponent question Further information is requested

Comments

@Nayte91
Copy link

Nayte91 commented Sep 9, 2024

Hello,

Part of the discussion about faceted search menu and results page combo (challenge 1), I'm searching to map different url parameters into one object.

In a symfony controller, you can welcome several query params in a single DTO with the MapQueryString attribute.

I wonder if we can, also in live components, group several query params into one property class?

In a live component, currently, when you have multiple query parameters to welcome, you must declare them one by one; then eventually, in a method, regroup them into an array or a DTO to do some logic. It can be cumbersome if you have a lot of potential query parameters. Plus, and I feel like it's more important, aligning behavior of component controllers (like a LC), with regular controllers is very important as it drastically lowers the mind burden and learning curve.

Context: you got a FacetedSearchMenu component, with multiple parameters that you want to reflect on url, and you group those in a FacetFilter DTO to pass to repository or whatever.

If I may help in any way,
Best regards,

@Nayte91 Nayte91 added the RFC label Sep 9, 2024
@Nayte91 Nayte91 changed the title [LiveComponent] Multiple url query params in one LiveProp [LiveComponent] Multiple URL query params in one LiveProp DTO Sep 29, 2024
@Nayte91
Copy link
Author

Nayte91 commented Oct 2, 2024

Possibilities:

  • new #[LiveQueryString] attribute?
  • make #[mapQueryString] works on a LC (without extending AbstractController?)
  • make url: true works on a DTO

@smnandre
Copy link
Member

smnandre commented Oct 5, 2024

I'm having doubt here... are DTO as LiveProp supposed to work already, even with URL ....

https://symfony.com/bundles/ux-live-component/current/index.html#supported-data-types

@Nayte91
Copy link
Author

Nayte91 commented Oct 5, 2024

Interesting point, did you achieve any minimal working example? Because I try since 1hour, and I always throw errors:

An exception has been thrown during the rendering of a template ("Warning: Undefined property: App\Component\ArticleResults::$name").

class ArticleResults
{
    use DefaultActionTrait;
    use ComponentToolsTrait;

//    #[LiveProp(url: true)]
//    public ?string $name = null;
//    #[LiveProp(url: true)]
//    public ?string $status= null;
//    #[LiveProp(writable: true, url: true)]
//    public ?int $page = 1;

    #[LiveProp(writable: true, url: true)]
    public ?Filter $filter = null;

    private ItemsPage $articlesPage;

    public functiondoStuff(): foo {...}
}

class Filter
{
    public function __construct(
        public ?int $page = 1,
        public ?string $name = null,
        public ?string $status = null,
    ) {}
}

I may miss something goofy here, but for me and based also on the DTO part of the doc, I can't make it work 😢

@smnandre
Copy link
Member

smnandre commented Oct 5, 2024

The error message you quote may indicate you try to mount "name" directly on ArticleResults in your template, no ?

@smnandre
Copy link
Member

smnandre commented Oct 5, 2024

search-filters.mov

@smnandre
Copy link
Member

smnandre commented Oct 5, 2024

(sorry for the support page ... i was working on something else 😅 )

@smnandre
Copy link
Member

smnandre commented Oct 5, 2024

<?php

namespace App\Twig;

use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
use Symfony\UX\LiveComponent\Attribute\LiveProp;
use Symfony\UX\LiveComponent\DefaultActionTrait;

#[AsLiveComponent]
final class Search
{
    use DefaultActionTrait;

    #[LiveProp(writable: ['search', 'size', 'type'], fieldName: 's', url: true)]
    public SearchFilters $filters;

    public function mount(): void
    {
        $this->filters ??= new SearchFilters();
    }

    public function getResults(): array
    {
        $results = [
            ['name' => 'Gorilla', 'size' => 'lg',  'type' => 'animal'],
            ['name' => 'Banana', 'size' => 'md',  'type' => 'fruit'],
            ['name' => 'Apple', 'size' => 'sm',  'type' => 'fruit'],
            ['name' => 'Lion', 'size' => 'lg',  'type' => 'animal'],
            ['name' => 'Strawberry', 'size' => 'sm',  'type' => 'fruit'],
            ['name' => 'Elephant', 'size' => 'lg',  'type' => 'animal'],
            ['name' => 'Orange', 'size' => 'md',  'type' => 'fruit'],
            ['name' => 'Tiger', 'size' => 'lg',  'type' => 'animal'],
            ['name' => 'Pineapple', 'size' => 'md',  'type' => 'fruit'],
            ['name' => 'Zebra', 'size' => 'lg',  'type' => 'animal'],
        ];

        $results = array_filter($results, function($result) {
            return $this->filters->search === '' || str_contains($result['name'], $this->filters->search);
        });

        $results = array_filter($results, function($result) {
            return $this->filters->size === '' || $this->filters->size === $result['size'];
        });

        $results = array_filter($results, function($result) {
            return $this->filters->type === '' || $this->filters->type === $result['type'];
        });

        return $results;
    }
}
<?php

namespace App\Twig;

use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
use Symfony\UX\LiveComponent\DefaultActionTrait;

final class SearchFilters
{
    public string $search = '';

    public string $type = '';

    public string $size = '';
}
<div{{ attributes }}>

    <h2>dump(this.filters)</h2>
    {{ dump(this.filters) }}

    <hr />

    <h2>Filters</h2>
    <div style="display: grid; grid-template-columns: 1fr 1fr 1fr;">

        <div>
            <label>Search</label>
            <input type="search" class="form-control" data-model="s[search]" />
        </div>

        <div>
            <label>Size</label>
            <select class="form-control" data-model="s[size]">
                <option value="">Size</option>
                <option value="sm">Small</option>
                <option value="md">Medium</option>
                <option value="lg">Large</option>
            </select>
        </div>

        <div>
            <label>Type</label>
            <label>
                <input type="radio" class="form-check" data-model="s[type]" value="animal" />
                Animal
            </label>
            <label>
                <input type="radio" class="form-check-input" data-model="s[type]" value="fruit" />
                Fruit
            </label>
        </div>
    </div>

    <hr />

    <div>
        <h2>Results</h2>

        {% for result in this.results %}
            <div id="{{ result.name }}" style="padding: 1rem 0; border-bottom: 2px dotted #eee;">
                <div class="d-flex justify-content-between">
                    <h5>{{ result.name }}</h5>
                    <span>{{ result.size }}</span>
                    <span>{{ result.type }}</span>
                </div>
            </div>
        {% endfor %}
    </div>

</div>

@smnandre
Copy link
Member

smnandre commented Oct 5, 2024

(please do not look too much at the code quality 😆 )

@smnandre smnandre added question Further information is requested and removed RFC labels Oct 5, 2024
@Nayte91
Copy link
Author

Nayte91 commented Oct 7, 2024

Thank you very much for taking the time to create example, I made some tries and here's my thoughts:

  • You're right, my error was that I still aimed for this.page instead of this.filter.page!
  • It also needs a mount() method where we instantiate the DTO: $this->filters ??= new SearchFilters();. Strange thing that we have #[PreMount] & #[PostMount] attributes, but no #[Mount] 🫢
  • Writable: true doesn't allow the properties inside the Filter dto to be written; you MUST explicitely call the properties in an array.
  • I may miss a point (again!) but it SEEMS that an 'onChange' event is not triggered when a property inside the DTO changes? Here hello doesn't appears when I try to change anyhow the page value on template:
class FacetMenu
{
    use DefaultActionTrait;

    #[LiveProp(writable: ['page'], onUpdated: 'emitChange', url: true)]
    public Filter $filter;

    public function mount() { $this->filter ??= new Filter; }

    public function emitChange(): void
    {
        dd('hello');
        $this->emit('facetSetted', ['filter' => $this->filter]); // for the next point
    }
}
  • The ComponentToolsTrait::emit() method only accepts array as $data; so I can't send the SearchFilter object like this. It must be insered into an array like ['filter' => $this->filters], that will be serialized and deserialized as an array. So it must be normalized as a SearchFilter object once in the other component:
class EventResults
{
    use DefaultActionTrait;

    #[LiveProp]
    public Filter $filter;

    #[LiveListener('facetSetted')]
    public function reload(#[LiveArg] array $filter): void 
    {
        dd($filter);
    }
}

All those constraints make the move quite complicated; I feel like using a DTO is way less intuitive, but I frankly don't know how to handle those issues 😶

@smnandre
Copy link
Member

smnandre commented Oct 7, 2024

You're very right on all these points.

So, now... would you want to help improving one ? And see where that leads ?

I can help, pin in the direction if you need, and probably help you figuring "what's not gonna stay that way" and "what is needed because this or that features"...

What do you think ?

@Nayte91
Copy link
Author

Nayte91 commented Oct 7, 2024

I definitely accept the challenge of open source!

  1. Adding a #[Mount] attribute: copy/pasting another attribute so trivia I guess (famous last words)
  2. Making #[LiveProp(writable: true)] works on a DTO seems mandatory and hard to counter argue?
  3. Making #[LiveProp(onChange: 'foo')] works on a DTO seems mandatory also?
  4. Having a mount() method to help a DTO to be created is sad, but I guess it's quite tricky to avoid,
  5. #[LiveArg] that could accept an object and auto normalize it, as a regular Symfony controller, is I think, a crossroads. My feeling as a newbie is that the regular SF controllers are very conveniants and intuitives, and the different behavior of LC is suboptimal and creates mind burden. So instead of fixing "LiveArg" I feel like LC should have a 'ControlerToolkit' addon, maybe a trait, or a abstract class, but tweaked for LCs (I know that you can extends AbstractController for a LC, but I guess a random method can't accept params normalization like controllers can), that will help to play with URLs and serlializations, because at the end the day, my whole problems/suggestions are basically "my LC is a little controller that need controller's power, please halp".

@smnandre
Copy link
Member

smnandre commented Oct 8, 2024

Would you agree to take some time to discuss them (slack / meet / anything / ?) when you have the time ?

Just quickly

  1. mount() method is called if exist, so not sure there really is a need here.

  2. I'm certain this will be more complicated than it seems right now... but yeah the DX is not optimal currently. Let's just keep in mind that LiveProp is a "solution" and maybe there is something else to consider.... like "what is a liveprop" and "do we need something else ?

  3. Yes, nothing to think of too much for now, as it entirely depends on the 2

  4. Not sure to understand here? You could instanciate it in the constructor, or before calling the component the first time... .but if you have only an empty class string ... is it really a liveprop you need, or a logger ?

  5. This is were i'm a bit :-/ A component should only react to a small (limited) set of methods (the actions), value changes, etc... so i would almost say the opposite, what would they use a request for ? a firewall ? a flash message.
    So here we see everyone has its own definition of what is a component, a statefull one, or not :) And i'm really not sure some implementation details we have right now are made for all these different visions. But all of them are legit and we need to clarify how we want to handle them and lower the frustrations.


At the same time of all your very relevant ideas and feedback... I also started to split more clearly Live and Twig component.. so i huess the work on Attributes, Metadata, Events is a must-have to clarify some behaviours, and free some place for new features :)

@Nayte91
Copy link
Author

Nayte91 commented Oct 8, 2024

I would be pleased to speak about this on slack for example; this week my planning is a wreck, I just write issues when I have time, but next week for sure.

Let me advocate for #[Mount] one last time 😅 as it is very small:

  1. #[PreMount] & #[PostMount] already exist,
  2. It helps code expressivity, as you may want to save one LOC and call your method mount(), but you can also want it expressive, call #[Mount] and create your method public function setPropFromRepository() (ok that's a very bad name),
  3. This SF pattern is common and exists everywhere, for example event listeners let you name your function 'onFoo()', or let you put an attribute with #[AsEventListener(event: 'foo')] and call your method however you want,
  4. Note that for completion, I don't know if you can currently create a postMount() function without attribute and make it behave like it has #[PostMount] 😄
  5. If it's done, I feel like we can create a small new 'LC events' of documentation, with those 3 and the tricks, make refs from the main page, and begin to slim down the doc 🥳

@Nayte91
Copy link
Author

Nayte91 commented Oct 8, 2024

Not sure to understand here? You could instanciate it in the constructor, or before calling the component the first time... .but if you have only an empty class string ... is it really a liveprop you need, or a logger ?

Point 4, about DTO initialization: I feel like we should left this one behind, unless someone comes with a veeery clever trick.

Maybe the most optimal version will be in the language, being able to declare properties like public SearchFilter $filter = new SearchFilter() I don't know if it's packed in 8.4 or will ever happen.

Anyway, it can be done also in constructor, I didn't think about it, nice trick 👌

@Nayte91
Copy link
Author

Nayte91 commented Oct 8, 2024

  1. This is were i'm a bit :-/ A component should only react to a small (limited) set of methods (the actions), value changes, etc...

Points 2 and 3 about LiveProp, needs to be clear about 5, 'general design' considerations. We agree that DX is not optimal, that TC and LC need more separation, and new features I am requesting (except Mount that is a bit stand alone) must integrate well with general design.

I will take time to read your links about the split, but let me give some food here:

  • Are you able to find much usecases where a LC does NOT behave like a controller? Because a like/dislike button, a form, a searchbar or an upload field are and behave exactly like a 'micro-controller' (having trouble with this term as it has a specific meaning in IT). My point is: I feel like a LC is deeply related to a controller role, and needs a lot of the regular, powerful SF controller's feature. So I may be biaised...
  • ... But I made some React, and it's a world where 'Components' are not a 'cool feature' but 'THE ONE AND ONLY PARADIGM'. So you encounter small, dumb components, big, smart components, problems with props (aww god if we can do better..), global ones, and so on. So my point is that I'm not sure a LC should be only small features, in fact the question is very good: can we give a role to LC that fits for both of us (and for everyone)? I would say packed and independant are the keys, so I agree with 'limited'. You should be able to share a LC between projects, or with friends. It should be simple to pack/bundle, with very identified files. I replaced all my stimulus controllers that made ajax calls, and destroyed the src/Controller/AjaxController we all have somewhere. I feel like this is the right use of TC/LCs?
  • Let's assume a world where we have 4 independant component types: anonymous components, static components, dynamic components, and controller components. In this world, you will have some discussions with people to maybe simplify SF UX, and factorize components; You will pretty surely end up with anonymous and static as one (TC), and dynamic & controller components as another one (LC). My point is that we intuitively assume that LCs have controllers features. But that's a scenario where you can totally take another way or fall on another conclusion!

Based on those highly debatable points, I feel like in an ideal world:

  • LCs have a AbstractLiveComponent class to extends.
  • This class should have LC specific tools, and shared tools with regular controllers.
  • SF controller's tooling code could be split in 2 traits: SharedControllerTrait and RouterSpecificTrait (assuming that some features from regular controller should NOT be used in a LC).
  • AbstractLiveComponent would use SharedControllerTrait and ComponentSpecificTrait.
  • LCs will be able to use #[MapQueryString] to deserialize arguments to a DTO, thanks to AbstractLiveComponent, along the MANY controller-powered-new features.
  • In an ideal world, 50% to 100% of the actual LC Form features should be assimiled to the regular Symfony Form component, as Form is basically a Component Factory, and the {{ form_start() }} function can totally carry the attributes.
  • SF Forms should have the symfonycasts/dynamic-forms's "dynamicForm" class as a native feature (fluent isLive() method, configureOptions() 'dynamic' boolean entry, ...), that will throw an error 'you need composer req stimulus in order to use this' if used in a regular project.
  • LCs documentation can cut the form part away to another page and refer to Form, cut the controller behaviors part away to another page and refer to routing.

That's some work and impossible wishes, but that's my statement for now 😆

@Nayte91
Copy link
Author

Nayte91 commented Oct 8, 2024

so i would almost say the opposite, what would they use a request for ? a firewall ? a flash message.

To answer about request, some reasons as I mix good & bad ideas:

  • to find base URL. Imagine a pagination component, with first/previous/next/last pages. It changes the number of 'page', but what about the baseURL if you use it on different pages? Here you either can import it with a mandatory prop, or deduct it from URL. It improves independency of the the component, at cost of some edge cases where 'no please it's not what I want!'.
  • to find previous page. Not exactly sure how symfony Security & firewall conf would behave if I make a LC from a login form; When I submit and successfully log in, I would like to be redirected to previous page, and therefor I will want to check in Request if I have the last route in session, or also send the original route with a prop.
  • ofc to catch URL parameters, until tooling allow to fetch them smoothly :) soon!

You're right for this part, I feel like none of my arguments are killer ones, and a good shaped LC can get rid of any Request need, on classical use cases (if some nerds want to inject it by constructor, so be it).

@smnandre
Copy link
Member

smnandre commented Oct 8, 2024

To be honest, I think we share the same vision on the vast majority of things here... i may have not expressed my personal ideas here with enough talent 😅

The remaining differences are purely philosophical and won't be a problem here :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LiveComponent question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants