livewire / livewire

A full-stack framework for Laravel that takes the pain out of building dynamic UIs.
MIT License
22.35k stars 1.56k forks source link

Using the model ID as a nested key is inconsistent #2179

Closed redbastie closed 3 years ago

redbastie commented 3 years ago

When I use this in a nested component:

@livewire('vehicles.delete', ['vehicle' => $vehicle->id], key($vehicle->id))

It is quite buggy. Sometimes the component won't refresh, or will outright throw a TracksRenderedChildren error.

However, if I use this instead:

@livewire('vehicles.delete', ['vehicle' => $vehicle->id], key(uniqid()))

Everything works flawlessly. Can someone explain?

joshhanley commented 3 years ago

@redbastie that is a morphdom issue. Below is an explaination I use for morphdom issues. I'd recommend in your instance using something like

key('vehicle' . $vehicle->id)

So general rule of thumb that I follow with morphdom issues, any blade conditional, wrap in a div, so morphdom knows anything inside that div is only the conditional stuff, and make sure the key is unique but duplicatable. What I mean by that is, I've seen people using timestamps to make the key unique, but you can't make it consistently unique, instead by using context name plus a unique ID from the DB ensures that you don't potentially have two keys say with the ID of 1 on the page

Hope this helps!

hilmanski commented 3 years ago

it also happen to me. In my case, when emit from child component to parent and just 'refresh' the data via listeners

 protected $listeners = [
        'userUpdated' => '$refresh'
    ];

Even i use 'user'.$user->id as key, it still not refreshing the data

There are two ways to resolve this issue (hacky way)

  1. uniqid as told above, but is it a good practice?

  2. i add the updated-column as key example i want to update username, i need to make a key from :key="$user->username"

then it can update the username, but other columns not.

Is it expected?

joshhanley commented 3 years ago

@hilmanski I have seen some issues where $refresh in a listener doesn't work properly.

Try making a method which has refresh in that and call that from the listener instead and see if it runs ok.

hilmanski commented 3 years ago

Thanks @joshhanley for the reply. I'll try. Do you mean using $refresh inside a method? or re fetch the data from DB, the second unfortunately not working in my case, i see it calls the right data when i dd($data), but its clearly the dom key issue.

I also didn't find documentation for $refresh on livewire's site. Do you know why/where?

*I think, it's good to let this issue open, so we aware there's still this dom issue which is important for livewire future. thanks!

[update] I think Caleb already aware of this https://github.com/livewire/livewire/issues/2044

joshhanley commented 3 years ago

@hilmanski yeah it's an issue, but it's not an issue, in the sense that morphdom needs things to be setup in a specific way (see my first comment) otherwise it breaks. If setup right though, works perfectly fine 😆

Yeah I meant the first option, $refresh in a method, but that may not work anyway if there is morphdom issues.

Can you share your code, so we can see if we can get it running?

artworkad commented 3 years ago

@joshhanley I am emitting an update event from a child component up to the parent list component. Trying to refresh the list.

Using this:

key('vehicle' . $vehicle->id)

the list is not refreshing. However using

key(time() . $vehicle->id)

does refresh the list.

My original post from the forum: https://forum.laravel-livewire.com/t/view-is-not-refreshing-when-the-collection-is-updated/3312/8

joshhanley commented 3 years ago

@artjomzab Livewire won't refresh child components as they are isolated from each other, so the way to get them to refresh is a bit of a hack, using keys. What happens is by changing the key, Livewire thinks it's actually a new dom element and so it deletes the old child component and launches a new one with the new key, and as such the new, refreshed, data.

Using the key like I suggested key('vehicle' . $vehicle->id) originally won't work for forcing the child component to refresh. That is why you will see people using keys like you suggested that has time() in it.

I would consider this a bad idea, purely because time changes every time the parent component re-renders. As such all child components that have that in the key will be deleted and replaced, and any dirty state they may have contained will be gone.

What I prefer to do now, is actually create a property on the parent component like public $uniqueKey; and populate that in your mount method $this->uniqueKey = uniqid();.

Then on your child components, set the key to something like this key('vehicle-' . $vehicle->id . '-'.$uniqueKey).

The last step then, is whenever you need to refresh all of the children (like in a listener), in the parent component you would just generate a new key in your method using $this->uniqueKey = uniqid();, which will change the signature of the key for all children, triggering morphdom to delete them all and create new ones.

Doing it this way prevents all components having to be re-initialised every time on the front end, but gives you the option to force changes when needed.

Hope this helps!

redbastie commented 3 years ago

I just use uniqid() for keys now. Works great.

quanengineering commented 3 years ago

It's a good strategy. Thanks @joshhanley.

joshhanley commented 3 years ago

@quanengineering no worries!

@redbastie I'm going to close this now. Feel free to reopen if you think your question isn't fully answered 🙂

squishythejellyfish commented 3 years ago

👋 Oh Hi! I'm Squishy, the friendly jellyfish that manages Livewire issues.

I see this issue has been closed.

Here in the Livewire repo, we have an "issues can be closed guilt-free and without explanation" policy.

If for ANY reason you think this issue hasn't been resolved, PLEASE feel empowered to re-open it.

Re-opening actually helps us track which issues are a priority.

Reply "REOPEN" to this comment and we'll happily re-open it for you!

(More info on this philosophy here: https://twitter.com/calebporzio/status/1321864801295978497)