filamentphp / filament

A collection of beautiful full-stack components for Laravel. The perfect starting point for your next app. Using Livewire, Alpine.js and Tailwind CSS.
https://filamentphp.com
MIT License
15.01k stars 2.43k forks source link

KeyValue field updates state from the second time #12824

Open finoghentov opened 2 weeks ago

finoghentov commented 2 weeks ago

Package

filament/filament

Package Version

3.2

Laravel Version

10.10

Livewire Version

3.4

PHP Version

8.1

Problem description

I have a simple resource editing form.

KeyValue::make('options'),
Select::make('type')
  ->options(array_column(BoostType::cases(), 'name'))
  ->afterStateUpdated(function (Component $component, Set $set) {
        $set('options', ['test' => rand(0, 1000)]);
  })
  ->nullable(false)
  ->live()

Key value update state only from the second time

The problem in this script

Initializing the component with NON empty value triggers updateState method which switches shouldUpdateRows flag to false

https://github.com/filamentphp/filament/blob/3.x/packages/forms/resources/js/components/key-value.js#L15

if (this.rows.length <= 0) {
    this.rows.push({ key: '', value: '' })
} else {
    this.updateState()
}

https://github.com/filamentphp/filament/blob/3.x/packages/forms/resources/js/components/key-value.js#L105

this.shouldUpdateRows = false

While first updating field state we trigger next code which switch flag to true https://github.com/filamentphp/filament/blob/3.x/packages/forms/resources/js/components/key-value.js#L72

if (!this.shouldUpdateRows) {
    this.shouldUpdateRows = true
    return
}

I've found that this was required to fix this bug but we need to find a better decision. Maybe something like this:

updateState: function () {
    let state = {}

    let keys = [];

    for (let i in this.rows) {
        if (
            keys.includes(this.rows[i].key) ||
            this.rows[i].key === '' ||
            this.rows[i].key === null
        ) {
            return;
        }

        keys.push(this.rows[i].key);
        state[this.rows[i].key] = this.rows[i].value
    }

    this.state = state
},

Remove shouldUpdate flag and do not update the state if we've found empty or duplicated keys. But maybe we should make a warning to make users understand that with duplicated keys state won't be updated

Expected behavior

I expect that changing the Select value will update the KeyValue state, but it works only If I change Select 2 times. It works only if you are updating the resource with some filled value in KeyValue.

Steps to reproduce

Reproduction repository

https://github.com/finoghentov/filament-key-value-bug

Relevant log output

No response