diff --git a/packages/forms/src/Components/Builder.php b/packages/forms/src/Components/Builder.php index 13a4ecea38..8c1e74819e 100644 --- a/packages/forms/src/Components/Builder.php +++ b/packages/forms/src/Components/Builder.php @@ -109,9 +109,9 @@ class Builder extends Field implements CanConcealComponents, HasEmbeddedView, Ha protected bool | Closure | null $shouldPartiallyRenderAfterActionsCalled = null; /** - * @var array | null + * @var array | null */ - protected ?array $cachedItems = null; + protected ?array $cachedItemsRawStateStructure = null; protected function setUp(): void { @@ -927,9 +927,17 @@ class Builder extends Field implements CanConcealComponents, HasEmbeddedView, Ha */ public function getItems(): array { - if ($this->cachedItems !== null) { - return $this->cachedItems; - } + return $this->getCachedDefaultChildSchemas(); + } + + /** + * @return array + */ + public function getDefaultChildSchemas(): array + { + $rawState = $this->getRawState(); + + $this->cachedItemsRawStateStructure = $this->getRawStateStructure($rawState); $blocks = []; @@ -937,7 +945,7 @@ class Builder extends Field implements CanConcealComponents, HasEmbeddedView, Ha $blocks[$block->getName()] = $block; } - return $this->cachedItems = collect($this->getRawState()) + return collect($rawState) ->filter(fn (array $itemData): bool => filled($itemData['type'] ?? null) && array_key_exists($itemData['type'], $blocks)) ->map( fn (array $itemData, $itemIndex): Schema => $blocks[$itemData['type']] @@ -951,18 +959,26 @@ class Builder extends Field implements CanConcealComponents, HasEmbeddedView, Ha } /** - * @return array + * Item schemas only depend on the raw state's structure - the item keys, their + * order, and each item's block type - since fields inside the items read their + * values from the live raw state. Comparing a structural fingerprint instead of + * the item values keeps this check cheap when it runs often, and avoids + * rebuilding the item schemas every time a value inside an item changes. */ - public function getDefaultChildSchemas(): array + protected function areCachedDefaultChildSchemasFresh(): bool { - return $this->getItems(); + return $this->cachedItemsRawStateStructure === $this->getRawStateStructure($this->getRawState()); } - public function clearCachedChildSchemas(): void + /** + * @return array + */ + protected function getRawStateStructure(mixed $rawState): array { - parent::clearCachedChildSchemas(); - - $this->cachedItems = null; + return array_map( + static fn (mixed $itemData): ?string => is_array($itemData) ? ($itemData['type'] ?? null) : null, + is_array($rawState) ? $rawState : [], + ); } public function getAddBetweenActionLabel(): string diff --git a/packages/forms/src/Components/Repeater.php b/packages/forms/src/Components/Repeater.php index df739b0301..da1bfe3c80 100644 --- a/packages/forms/src/Components/Repeater.php +++ b/packages/forms/src/Components/Repeater.php @@ -61,9 +61,9 @@ class Repeater extends Field implements CanConcealComponents, HasEmbeddedView, H protected ?Collection $cachedExistingRecords = null; /** - * @var array | null + * @var array | null */ - protected ?array $cachedItems = null; + protected ?array $cachedItemsRawStateStructure = null; protected string | Closure | null $orderColumn = null; @@ -848,9 +848,17 @@ class Repeater extends Field implements CanConcealComponents, HasEmbeddedView, H */ public function getItems(): array { - if ($this->cachedItems !== null) { - return $this->cachedItems; - } + return $this->getCachedDefaultChildSchemas(); + } + + /** + * @return array + */ + public function getDefaultChildSchemas(): array + { + $rawState = ($this->getRawState() ?? []); + + $this->cachedItemsRawStateStructure = array_map(is_array(...), $rawState); $relationship = $this->getRelationship(); @@ -858,7 +866,7 @@ class Repeater extends Field implements CanConcealComponents, HasEmbeddedView, H $items = []; - foreach ($this->getRawState() ?? [] as $itemKey => $itemData) { + foreach ($rawState as $itemKey => $itemData) { $items[$itemKey] = $this ->getChildSchema() ->statePath($itemKey) @@ -868,15 +876,21 @@ class Repeater extends Field implements CanConcealComponents, HasEmbeddedView, H ->getClone(); } - return $this->cachedItems = $items; + return $items; } /** - * @return array + * Item schemas only depend on the raw state's structure - the item keys, their + * order, and whether each item holds an array - since fields inside the items + * read their values from the live raw state. Comparing a structural fingerprint + * instead of the item values keeps this check cheap when it runs often, and + * avoids rebuilding the item schemas every time a value inside an item changes. + * The existing records that relationship items embed are not observable through + * the raw state, so `clearCachedExistingRecords()` clears the items explicitly. */ - public function getDefaultChildSchemas(): array + protected function areCachedDefaultChildSchemasFresh(): bool { - return $this->getItems(); + return $this->cachedItemsRawStateStructure === array_map(is_array(...), $this->getRawState() ?? []); } public function getAddActionLabel(): string @@ -978,11 +992,6 @@ class Repeater extends Field implements CanConcealComponents, HasEmbeddedView, H public function saveToRelationship(): void { - // The raw state may have been mutated through an ancestor schema (e.g. `Schema::rawState()`), - // which clears that ancestor's cached child schemas but not this component's. Rebuild the - // memoized items so the save reflects the current state rather than a stale set. - $this->cachedItems = null; - $state = $this->getState(); if (! is_array($state)) { @@ -1326,14 +1335,10 @@ class Repeater extends Field implements CanConcealComponents, HasEmbeddedView, H public function clearCachedExistingRecords(): void { $this->cachedExistingRecords = null; - $this->cachedItems = null; - } - public function clearCachedChildSchemas(): void - { - parent::clearCachedChildSchemas(); - - $this->cachedItems = null; + // Items embed the existing records, which the raw state snapshot cannot + // observe, so they must be cleared explicitly alongside the records. + $this->clearCachedChildSchemas(); } /** diff --git a/packages/infolists/src/Components/RepeatableEntry.php b/packages/infolists/src/Components/RepeatableEntry.php index f77f88b659..e6cd36a8d9 100644 --- a/packages/infolists/src/Components/RepeatableEntry.php +++ b/packages/infolists/src/Components/RepeatableEntry.php @@ -26,10 +26,7 @@ class RepeatableEntry extends Entry implements HasEmbeddedView */ protected array | Closure | null $tableColumns = null; - /** - * @var array | null - */ - protected ?array $cachedItems = null; + protected mixed $cachedItemsState = null; /** * Configure table columns for display @@ -66,13 +63,19 @@ class RepeatableEntry extends Entry implements HasEmbeddedView */ public function getItems(): array { - if ($this->cachedItems !== null) { - return $this->cachedItems; - } + return $this->getCachedDefaultChildSchemas(); + } + + /** + * @return array + */ + public function getDefaultChildSchemas(): array + { + $this->cachedItemsState = $state = ($this->getState() ?? []); $containers = []; - foreach ($this->getState() ?? [] as $itemKey => $itemData) { + foreach ($state as $itemKey => $itemData) { $container = $this ->getChildSchema() ->getClone() @@ -88,22 +91,12 @@ class RepeatableEntry extends Entry implements HasEmbeddedView $containers[$itemKey] = $container; } - return $this->cachedItems = $containers; + return $containers; } - /** - * @return array - */ - public function getDefaultChildSchemas(): array + protected function areCachedDefaultChildSchemasFresh(): bool { - return $this->getItems(); - } - - public function clearCachedChildSchemas(): void - { - parent::clearCachedChildSchemas(); - - $this->cachedItems = null; + return $this->cachedItemsState === ($this->getState() ?? []); } public function toEmbeddedHtml(): string diff --git a/packages/schemas/src/Components/Concerns/HasChildComponents.php b/packages/schemas/src/Components/Concerns/HasChildComponents.php index d9f86bf7a5..6f5be60304 100644 --- a/packages/schemas/src/Components/Concerns/HasChildComponents.php +++ b/packages/schemas/src/Components/Concerns/HasChildComponents.php @@ -79,11 +79,11 @@ trait HasChildComponents public function getChildSchema($key = null): ?Schema { if (filled($key) && ! array_key_exists($key, $this->childComponents)) { - return ($this->cachedDefaultChildSchemas ??= $this->getDefaultChildSchemas())[$key] ?? null; + return $this->getCachedDefaultChildSchemas()[$key] ?? null; } - if (filled($key) && array_key_exists($key, $this->cachedDefaultChildSchemas ??= $this->getDefaultChildSchemas())) { - return $this->cachedDefaultChildSchemas[$key]; + if (filled($key) && array_key_exists($key, $cachedDefaultChildSchemas = $this->getCachedDefaultChildSchemas())) { + return $cachedDefaultChildSchemas[$key]; } $key ??= 'default'; @@ -162,7 +162,7 @@ trait HasChildComponents } return [ - ...(array_key_exists('default', $this->childComponents) ? ($this->cachedDefaultChildSchemas ??= $this->getDefaultChildSchemas()) : []), + ...(array_key_exists('default', $this->childComponents) ? $this->getCachedDefaultChildSchemas() : []), ...array_reduce( array_keys($this->childComponents), function (array $carry, string $key): array { @@ -199,6 +199,29 @@ trait HasChildComponents return ['default' => $this->getChildSchema()]; } + /** + * @return array + */ + protected function getCachedDefaultChildSchemas(): array + { + if (($this->cachedDefaultChildSchemas !== null) && $this->areCachedDefaultChildSchemasFresh()) { + return $this->cachedDefaultChildSchemas; + } + + return $this->cachedDefaultChildSchemas = $this->getDefaultChildSchemas(); + } + + /** + * Components whose child schemas are derived from state, such as repeaters, + * can override this method to compare the current state against a snapshot + * taken when the cache was built, so that the cache invalidates itself when + * the state changes, without an explicit `clearCachedChildSchemas()` call. + */ + protected function areCachedDefaultChildSchemasFresh(): bool + { + return true; + } + public function clearCachedChildSchemas(): void { $this->cachedDefaultChildSchemas = null; diff --git a/tests/src/Forms/Components/BuilderTest.php b/tests/src/Forms/Components/BuilderTest.php index 1b99673092..cecd53a94d 100644 --- a/tests/src/Forms/Components/BuilderTest.php +++ b/tests/src/Forms/Components/BuilderTest.php @@ -5,6 +5,8 @@ use Filament\Actions\Testing\TestAction; use Filament\Forms\Components\Builder; use Filament\Forms\Components\Repeater; use Filament\Forms\Components\TextInput; +use Filament\Schemas\Components\Section; +use Filament\Schemas\Components\Utilities\Set; use Filament\Schemas\Schema; use Filament\Support\Enums\Alignment; use Filament\Tests\Fixtures\Livewire\Livewire; @@ -1857,3 +1859,65 @@ class TestComponentWithBuilderFilledFromMount extends Livewire ->statePath('data'); } } + +it('rebuilds blocks after an `afterStateUpdated` hook uses `$set()` on an ancestor\'s state path', function (): void { + livewire(BuilderInStatePathAncestorSetByHook::class) + ->assertSeeText('First block type') + ->assertDontSeeText('Second block type') + ->set('data.trigger', 'anything') + ->assertSeeText('Second block type'); +}); + +class BuilderInStatePathAncestorSetByHook extends Livewire +{ + public function mount(): void + { + $this->form->fill([ + 'trigger' => null, + 'group' => [ + 'blocks' => [ + ['type' => 'one', 'data' => ['foo' => 'A']], + ], + ], + ]); + } + + public function form(Schema $form): Schema + { + return $form + ->schema([ + // The `Section` is deliberately registered before the `trigger` field, so that + // the `afterStateUpdated` walk traverses it, and the `Builder` caches its + // items, before the `trigger` field's hook runs `$set()`. + Section::make('Blocks') + ->statePath('group') + ->schema([ + Builder::make('blocks') + ->addable(false) // Without the add action, its block picker does not render every block type's label, so the assertions below can rely on the rendered block headers alone. + ->blocks([ + Builder\Block::make('one') + ->label('First block type') + ->schema([ + TextInput::make('foo'), + ]), + Builder\Block::make('two') + ->label('Second block type') + ->schema([ + TextInput::make('bar'), + ]), + ]), + ]), + TextInput::make('trigger') + ->live() + ->afterStateUpdated(function (Set $set): void { + $set('group', [ + 'blocks' => [ + ['type' => 'one', 'data' => ['foo' => 'A']], + ['type' => 'two', 'data' => ['bar' => 'B']], + ], + ]); + }), + ]) + ->statePath('data'); + } +} diff --git a/tests/src/Forms/Components/RepeaterTest.php b/tests/src/Forms/Components/RepeaterTest.php index 428d10db51..c19fab375f 100644 --- a/tests/src/Forms/Components/RepeaterTest.php +++ b/tests/src/Forms/Components/RepeaterTest.php @@ -3082,6 +3082,67 @@ class RepeaterWithMutateBeforeCreateReturnsNull extends Component implements Has } } +it('rebuilds items after an `afterStateUpdated` hook uses `$set()` on an ancestor\'s state path', function (): void { + livewire(RepeaterInStatePathAncestorSetByHook::class) + ->assertSeeText('Original item') + ->set('data.trigger', 'anything') + ->assertSeeText('Added item'); +}); + +class RepeaterInStatePathAncestorSetByHook extends Component implements HasSchemas +{ + use InteractsWithSchemas; + + public ?array $data = []; + + public function mount(): void + { + $this->form->fill([ + 'trigger' => null, + 'group' => [ + 'items' => [ + ['name' => 'Original item'], + ], + ], + ]); + } + + public function form(Schema $form): Schema + { + return $form + ->schema([ + // The `Section` is deliberately registered before the `trigger` field, so that + // the `afterStateUpdated` walk traverses it, and the `Repeater` caches its + // items, before the `trigger` field's hook runs `$set()`. + Section::make('Items') + ->statePath('group') + ->schema([ + Repeater::make('items') + ->itemLabel(static fn (?array $state): string => $state['name'] ?? '') + ->schema([ + TextInput::make('name'), + ]), + ]), + TextInput::make('trigger') + ->live() + ->afterStateUpdated(function (Set $set): void { + $set('group', [ + 'items' => [ + ['name' => 'Original item'], + ['name' => 'Added item'], + ], + ]); + }), + ]) + ->statePath('data'); + } + + public function render(): View + { + return view('livewire.form'); + } +} + class RepeaterWithTranslatableContentDriver extends Component implements HasActions, HasSchemas { use InteractsWithActions; diff --git a/tests/src/Infolists/Components/RepeatableEntryTest.php b/tests/src/Infolists/Components/RepeatableEntryTest.php index 2bdf23a882..452ec99df4 100644 --- a/tests/src/Infolists/Components/RepeatableEntryTest.php +++ b/tests/src/Infolists/Components/RepeatableEntryTest.php @@ -2,6 +2,10 @@ namespace Filament\Tests\Infolists\Components; +use Filament\Actions\Action; +use Filament\Actions\Concerns\InteractsWithActions; +use Filament\Actions\Contracts\HasActions; +use Filament\Actions\Testing\TestAction; use Filament\Forms\Components\TextInput; use Filament\Infolists\Components\RepeatableEntry; use Filament\Infolists\Components\RepeatableEntry\TableColumn; @@ -529,6 +533,69 @@ describe('relationships', function (): void { } } + it('does not render a related record\'s row after an `Action` deletes it, without a page refresh', function (): void { + $user = User::factory() + ->has(Post::factory()->count(2)->sequence( + ['title' => 'Delete me without a refresh'], + ['title' => 'Keep me rendered'], + ), 'posts') + ->create(); + + $component = livewire(TestRelationshipRepeatableEntryWithDeleteAction::class, ['user' => $user]) + ->assertSeeText('Delete me without a refresh') + ->assertSeeText('Keep me rendered') + ->callAction(TestAction::make('deletePost')->schemaComponent('posts.0.title')); + + expect(Post::query()->where('title', 'Delete me without a refresh')->exists())->toBeFalse(); + expect($user->posts()->count())->toBe(1); + + $component + ->assertSeeText('Keep me rendered') + ->assertDontSeeText('Delete me without a refresh'); + }); + + class TestRelationshipRepeatableEntryWithDeleteAction extends Component implements HasActions, HasSchemas + { + use InteractsWithActions; + use InteractsWithSchemas; + + public User $user; + + public function mount(User $user): void + { + $this->user = $user; + } + + public function infolist(Schema $schema): Schema + { + return $schema + ->record($this->user) + ->components([ + RepeatableEntry::make('posts') + ->schema([ + TextEntry::make('title') + ->registerActions([ + Action::make('deletePost') + ->action(function (Post $record): void { + $record->delete(); + + $this->user->setRelation('posts', $this->user->posts()->get()); + }), + ]), + ]), + ]); + } + + public function render(): string + { + return <<<'BLADE' +
+ {{ $this->infolist }} +
+ BLADE; + } + } + }); it('correctly asserts entry state within `RepeatableEntry` using `assertSchemaComponentStateSet()`', function (): void {