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

[Bug]: Sorting breaks table when using LivewireComponentColumn #1950

Open
maikezan opened this issue Sep 17, 2024 · 7 comments
Open

[Bug]: Sorting breaks table when using LivewireComponentColumn #1950

maikezan opened this issue Sep 17, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@maikezan
Copy link

What happened?

Clicking a field column header with sorting enabled, should apply the requested sort with no errors. Instead the sort is applied but then the datatable breaks.
Filtering and columns visibility seems to work fine.

How to reproduce the bug

Build a datatable, use a LivewireComponentColumn as one of the columns.
Click on any orderable field. Data is sorted but the table breaks.
This is more evident if you enable bulk actions. because some checkboxes disappears like the screenshots below

image

In browser console there is the following js, livewire related error:

Uncaught Component already initialized

Package Version

3.4.16

PHP Version

8.3.x

Laravel Version

11.21.0

Alpine Version

3.14.1

Theme

Tailwind 3.x

Notes

No response

Error Message

Uncaught Component already initialized

@maikezan maikezan added the bug Something isn't working label Sep 17, 2024
@maikezan
Copy link
Author

maikezan commented Sep 17, 2024

Small but important update, also linking to this other issue: #1644

the issue seems to be resolved by providing a unique key in the attributes of the LivewireComponentColumn. Looking at the source code i thought that the key was already being passed by the main class, instead it needs to be passed through the attributes:

 LivewireComponentColumn::make(__('Something'), 'some_field')
                  ->component('some_component_name')->attributes(function ($columnValue, $row) {
                      return [
                          ......
                          'key' => '<something_unique>'
                      ];
                  })

@lrljoe
Copy link
Collaborator

lrljoe commented Sep 18, 2024

@maikezan - so by default, it should receive a wire key of

$row->{$row->getKeyName()}

However - your approach is absolutely fine.

Can I ask what you're using a Livewire Component Column for? It's certainly an edge case!

@maikezan
Copy link
Author

Hi @lrljoe !
Thanks for feedback.
Aboute the wire key, for some reason, it didn't work the way it was expected, causing the mentioned issues when refreshing the table .
Only by explicitely passing the key attribute we were able to make it work.

About why we chose the livewire column:
this particular datatable have data that shows live state of devices and stuff related to them.
Some field needs to be updated frequently as a result of some external event that we broadcast via websocket channels (a device calls an api endpoint for example, and that trigger some event that changes some data , and we need to show it in real time when it happens).
Refreshing the entire table in these kind of scenario may be too much so we ended up trying the livewire component approach.
While we understand the performance issue at larger scale, it may be good enough for now since we don't have many many records to handle.

@lrljoe
Copy link
Collaborator

lrljoe commented Sep 20, 2024

Hi @lrljoe ! Thanks for feedback. Aboute the wire key, for some reason, it didn't work the way it was expected, causing the mentioned issues when refreshing the table . Only by explicitely passing the key attribute we were able to make it work.

About why we chose the livewire column: this particular datatable have data that shows live state of devices and stuff related to them. Some field needs to be updated frequently as a result of some external event that we broadcast via websocket channels (a device calls an api endpoint for example, and that trigger some event that changes some data , and we need to show it in real time when it happens). Refreshing the entire table in these kind of scenario may be too much so we ended up trying the livewire component approach. While we understand the performance issue at larger scale, it may be good enough for now since we don't have many many records to handle.

That seems like a legitimate and typical use case for a Livewire Component Column, so no moans from me!

Now, you COULD do that listening via AlpineJS., keeping things client-side, and smoother, which at scale would be better, but if you're dealing with a production instance, which has <5,000 concurent users on the site, then this really won't make too much of a difference

I will definitely take a look at the Livewire Component Column to look at what's going awry, as it should absolutely "just work".

Sharing a "working" and "non-working" example helps immensely here. (feel free to ping me on the official Discord should you wish).

@me-julian
Copy link
Contributor

I ran into the same problem. I also have another issue which should perhaps be a separate issue, so I won't get into it here, though it might be related.

This is my first time using Livewire so bear with me if I get anything wrong.

Based on my reading of the Livewire docs on dynamic child components and messing around, I think I might have figured out at least part of what's happening here.

In my case, I figured out that $row->{$row->getKeyName()} was evaluating to null. Looking at the code, getKeyName just gets the primary key of your table's model. My primary key was the table id, but I didn't have any column for it in my data table, so it wasn't included in the select and so actually a property set on $row. In my case I used the setAdditionalSelects method in my table configuration to add the id back in without needing a column.

I figured this out because Livewire uses the key to track the child components on the main table. You can see this in the HTML. If the key is null, my main table shows something like this within the wire:snapshot attribute:

"children":[["div","VhljP58VW9fEs2IlKeaT"],["div","Scu1l7u9d0sgl3UbTFwg"] . . , ]

while if it's set properly it looks like this:

"children":{"fakeid1":["div","CG10F2f3ma9OHsz3HbCy"],"fakeid2":["div","6eOnycjrfC25J8vuqFpN"] . . , }

I'm still setting the key manually in the livewire component column's attributes because once it does get the actual id instead of NULL, I get an error like "Undefined constant "fakeid6".

This is what the dynamic components looks like inside my view cache, where the error is occuring:

<livewire:dynamic-component :component="$component" :coachingApplicantStatuses="$coachingApplicantStatuses" :status_id="$status_id" :applicant_id="$applicant_id" :wire:key="fakeid6" />

As far as I can tell, this is because the attributes prefixed with " : " are dynamic, so it's trying to resolve "fakeid6" like a variable (or const in this case) instead of read it as a string.

Adding the key in the column's attributes stops this error. Then I get this:

<livewire:dynamic-component :component="$component" :coachingApplicantStatuses="$coachingApplicantStatuses" :status_id="$status_id" :applicant_id="$applicant_id" :key="$key" :wire:key="fakeid6" />

As far as I can see in the docs I linked above, they don't mention adding :wire:key to dynamic child components at all, just :key. So maybe this is how it's being set correctly? The ":key" creates a random id, which you can see on the HTML tag as wire:id, so wire:key is probably redundant?

I don't actually see any wire:key in the HTML, so as far as I can tell that :wire:key="fakeid6" is still failing to resolve, just not erroring anymore.

With all that said, now I'm getting "Component not found: <wire:id of one of the dynamic components>" errors somewhat randomly when I interact with the table like changing filters. So I'm not sure if just setting the :key attribute is a real fix.

I made another issue with a sample repo yesterday, so I could probably add some examples of this stuff in there later if that helps.

Copy link

stale bot commented Nov 4, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Nov 4, 2024
@lrljoe lrljoe removed the wontfix This will not be worked on label Nov 23, 2024
@lrljoe
Copy link
Collaborator

lrljoe commented Nov 23, 2024

Removing wontfix for a review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants