Fix $watch and Support nested properties

[ahmedkandel]

Closes #279

Be more specific on watch callbacks to fire only on correct level. Add support for nested properties using dot-delimited paths like Vue.

It still needs to support nested objects and complex structures.

[ahmedkandel]

$watch has some issues explained on #279 due to how ObservableMembrane works, As we can't get the parents of the mutated property and match them with the watched property.

My proposal here is to invert the matching mechanism by getting the target for each watcher then match (target, key) of the mutation with (target, key) of the watcher.

Also adding dot-delimited paths like in Vue to watch nested properties e.g. $watch('a.b.c', value => console.log(value))

The test explains how it works.

$watch will still only support simple data types, I will add more commits to make it support complex data types.

[calebporzio]

Thanks! I changed the implementation a bit if you care to check it out.

[SimoTod]

@calebporzio I may be mistaken but i think that, if your component defines a watcher for a property at root level using the same name of the nested property, Alpine will call the wrong watcher there.

For example

        <div x-data="{ foo: { bar: 'baz', bob: 'lob' }, bar: 'baz' }" x-init="
            $watch('foo.bar', value => alert('ok'));
            $watch('bar', value => alert('ops'));
        ">
            <h1 x-text="foo.bar"></h1>
            <button x-on:click="foo.bar = 'law'"></button>
        </div>

P.s. it's worth noting that the first implementation doesn't scale well (it's O(n) rather then O(1) since we cycle on all the watchers every time) but I assume it's the best we can do and it's not too bad since people won't define thousands of watchers for the same component.

[ahmedkandel]

@calebporzio I like the new implementation but it will not fix the $watch issue here:

<div x-data="{ foo: 'bar', subObj: { foo: 'baz' } }" x-init="$watch('foo', value => console.log('$watch callback', value))">
    <button x-on:click="foo = 'qux'">Change 'foo' property</button>
    <button x-on:click="subObj.foo = 'quux'">Change 'subObj.foo' property</button>     
</div>

[ahmedkandel]

ooh, have just seen @SimoTod comment 👏 . yes correct it is not the best solution but that is what we can do for now. watchers will not be thousands so I don't think it will have any performance issue. Plus it will make support deep watcher harder than the first implementation. I will learn from your implementation and refactor the mine also add the support for the deep watcher.

[SimoTod]

@ahmedkandel Yeah, we posted almost at the same time. :)
A mix between your implementation and Caleb's should do the trick.

I don't know how you guys feel about that but we could also have something like

            valueObserved(target, key) {
                if(typeof target[key] === 'object') {
                    const obj = membrane.objectGraph.get(target[key])
                    const parent = membrane.objectGraph.get(target)
                    const prefix = parent && parent['$prefix'] ? parent['$prefix'] : ''
                    if(obj) {
                        obj['$prefix'] = prefix+key+"."
                    }
                }
            },

to the ObservableMembrane configuration and add

                const prefix = membrane.objectGraph.get(target)['$prefix'];
                if (prefix) {
                    key = prefix+key
                }

at the top of valueMutated

the "so-so" bit is that we would be using objectGraph which is an internal api so probably we shouldn't. :)

[ahmedkandel]

😉 I was working on something similar but for deep watcher:

export function foundUnderObjectGraph(needle, object, context) {
    if (needle === object) return true

    Object.getOwnPropertyNames(object).filter(
        property => typeof object[property] === 'object' && !property.startsWith('$')
    ).forEach(key => {
        if (foundUnderObjectGraph(needle, context.membrane.unwrapProxy(object[key]), context)) return true
    })

    return false
}

It is a draft. I didn't know about objectGraph API. Maybe it is better to avoid undocumented API and use our own. What do you think?

[SimoTod]

Yeah, that was the big problem. If they decide to change it (unlikely but possible) it would break. It's a shame because it's a weakmap so accessing the object is really performant and it doesn't depend on the array size, but yeah, let's leave it.

[ahmedkandel]

Added a new PR #294 that also support deep watch with codepen to try