-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Closure inside getStateUsing() called 3 times per row #14852
Comments
Hey @btxtiger! We're sorry to hear that you've hit this issue. 💛 However, it looks like you forgot to fill in the reproduction repository URL. Can you edit your original post and then we'll look at your issue? We need a public GitHub repository which contains a Laravel app with the minimal amount of Filament code to reproduce the problem. Please do not link to your actual project, what we need instead is a minimal reproduction in a fresh project without any unnecessary code. This means it doesn't matter if your real project is private / confidential, since we want a link to a separate, isolated reproduction. That would allow us to download it and review your bug much easier, so it can be fixed quicker. Please make sure to include a database seeder with everything we need to set the app up quickly. |
Thank you for providing reproduction steps! Reopening the issue now. |
i think you should use formatStateUsing instead of getStateUsing() |
@Cybrarist this does not work, since the state of the column should be any query or value (probably also a result of an HTTP request, Cache, other service, ...), and not dependant on a model column. |
@btxtiger oh ok, i get what you mean, then why not doing it like the following $table->modifyQueryUsing(function (Builder $query) {
$query->withCount('permissions');
}) and then you use 'permissions_count' in your TextColumn. |
@Cybrarist yes this would be a workaround for this specific example, but would not solve the actual problem. Tables\Columns\TextColumn::make('permission_granted')
->label('Permissions')
->badge()
->getStateUsing(function (User $user) {
return Http::get('https://dummy-auth-provider.com/api/check-permissions', ['uuid' => $user->uuid])
->json('permissions_granted') ? 'Granted' : 'Not Granted';
}); In this case, the closure is still executed 3 times, and thus make 3 https requests * number of rows, for 1 page load |
Seems a very bad idea to get the state of a column by doing an HTTP call for each one. |
Yeah, but still not targeting the issue. The issue is 3 redundant calls of a function where only one should be. Filament 3 became so much slower (less snappy) than v2, and I suspect this is one issue |
have the same problem here |
can confirm this (using function to calculate some values) - is run 3 times and output triples the value... |
After reviewing the code, it seems the issue might be associated with the ->disabledClick(true) I hope this helps as a temporary workaround until a proper solution is found. thanks. |
Would caching the state work? Something like modifying protected array $cachedStatePerRecordId = [];
protected function getRecordId(): mixed
{
return $this->getRecord()?->getKey();
}
public function getState(): mixed
{
if (! $this->getRecord()) {
return null;
}
if (array_key_exists($this->getRecordId(), $this->cachedStatePerRecordId)) {
return $this->cachedStatePerRecordId[$this->getRecordId()];
}
$state = ($this->getStateUsing !== null) ?
$this->evaluate($this->getStateUsing) :
$this->getStateFromRecord();
if (is_string($state) && ($separator = $this->getSeparator())) {
$state = explode($separator, $state);
$state = (count($state) === 1 && blank($state[0])) ?
[] :
$state;
}
if (blank($state)) {
$state = $this->getDefaultState();
}
$this->cachedStatePerRecordId[$this->getRecordId()] = $state;
return $state;
} |
@robinmoisson Nice approach, however it sounds for me like a workaround, since it is still unclear why the execution happens three times and if it still might affect other unexposed scenarios that throttles the overall performance. |
Thanks for the feedback @btxtiger. So I dove deeper into this, and it looks like the problem is this filament/packages/tables/src/Columns/Concerns/CanBeCopied.php Lines 12 to 22 in 43f74db
It is called in the blade template when building the column before building the cell, I don't see an easy way to inject the state over here: filament/packages/tables/resources/views/index.blade.php Lines 1145 to 1153 in af36dec
The state is also computed in the
So I'm not sure how to avoid this double call. As to how to go from 3 calls to 2 calls, when I look at the compiled blade code it looks like using an expression in a blade parameter calls it twice: once to create the component and once to pass attributes to it? If I turn @php
$isClickDisabled = $column->isClickDisabled() || $isReordering;
@endphp
<x-filament-tables::columns.column
:column="$column"
:is-click-disabled="$isClickDisabled"
...
/> then I only have two calls. I didn't know this about blade, this could probably be a legitimate PR for a perf improvement to cut computing time by 30% - which I guess is better than nothing. |
Package
filament/tables
Package Version
v3.2.124
Laravel Version
v11.33.2
Livewire Version
default
PHP Version
8.3.13
Problem description
When using
![image](https://private-user-images.githubusercontent.com/15804690/388156797-3007e23d-ba5d-4c07-a331-563f2c1c8d2f.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk3MDQ1MjQsIm5iZiI6MTczOTcwNDIyNCwicGF0aCI6Ii8xNTgwNDY5MC8zODgxNTY3OTctMzAwN2UyM2QtYmE1ZC00YzA3LWEzMzEtNTYzZjJjMWM4ZDJmLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTYlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE2VDExMTAyNFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWZlZjFkYjcwMGI4OWY2ZTRiYWU2NWQ5NDNkMGM5NGZmOThiYzQzYTNmNDBhYjI1Y2E2YzViYWY5MzJiMjU0MzkmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.tIbXJVSaiglgoTLTe1yU_iLqpVN4vnMELW1VKOPHIV0)
->getStateUsing()
to fill a column with a custom value, the Closure seems to be called 3 times per row!If this closure contains a custom query, this will lead to triple query execution per row!
![Bildschirmfoto 2024-11-20 um 16 16 22](https://private-user-images.githubusercontent.com/15804690/388157289-bf33e0f6-b75c-4fe8-bc1a-7d73d03b0832.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk3MDQ1MjQsIm5iZiI6MTczOTcwNDIyNCwicGF0aCI6Ii8xNTgwNDY5MC8zODgxNTcyODktYmYzM2UwZjYtYjc1Yy00ZmU4LWJjMWEtN2Q3M2QwM2IwODMyLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTYlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE2VDExMTAyNFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWUxYzhjMGEyNTAzNTE4Y2QwMzQ3NjgxYWUyYjEzNTk5NjczMzk1ZjdhZmU2OTFmMzczZTQzOTc4M2QzNzFkZjAmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.jpu8Z59TXum1yStO67O5ws0MjEn7aPfPne2HRgbBqCs)
Expected behavior
The Closure should be executed once on page load
Steps to reproduce
Example Column:
Reproduction repository (issue will be closed if this is not valid)
https://github.com/btxtiger/filament-getstateusing-bug-14852
Relevant log output
No response
Donate 💰 to fund this issue
The text was updated successfully, but these errors were encountered: