hybridsjs / hybrids

Extraordinary JavaScript UI framework with unique declarative and functional architecture
https://hybrids.js.org
MIT License
3.03k stars 85 forks source link

sync() with nested models is broken #256

Closed Qsppl closed 3 months ago

Qsppl commented 3 months ago

Expected Behavior

I am calling two models to sync and I expect that the sync will not call the store.get / store.set methods of that model's store.

example1: https://codepen.io/qsppl/pen/oNRLqLz

import { define, html, store } from "https://esm.sh/hybrids@8.2.23";

const Project = {
    id: true,
    feature: store.ref(() => Feature),
    [store.connect]: async (id) => {
        throw new Error("The Project model must not be loaded from the API!");
    }
};

const Feature = {
    id: true,
    project: store.ref(() => Project),
    [store.connect]: (id) => {
        throw new Error("The Feature model must not be loaded from the API!");
    }
};

store.sync(Feature, {
    id: "0",
    project: {
        id: "321",
        feature: "2"
    }
});
store.sync(Feature, {
    id: "1",
    project: {
        id: "321",
        feature: "3"
    }
});

Observed Behavior

I call two models to sync and the store.get method of that model's store is called once.

Note

If you do not call the second model to synchronize, then store.get will not be called: example2: https://codepen.io/qsppl/pen/abrZYWv?editors=0010

store.sync(Feature, {
    id: "0",
    project: {
        id: "321",
        feature: "2"
    }
});
// store.sync(Feature, {
//     id: "1",
//     project: {
//         id: "321",
//         feature: "3"
//     }
// });
smalluban commented 3 months ago

Looks like a valid bug, I'll look at this.

v9.0.0 release waits to be released with some breaking changes, so it might take some time to update when the bugfix is there. I think you can avoid the bug for now.

The error only occurs in your example when a feature with a set id does not exist. You can avoid an error if you define a feature model with the id set to "3" before the commented code. Then, the get method won't be called (the model will be taken from the memory cache).

Qsppl commented 3 months ago

Thank you. Take your time, for me this is a non-critical bug.

smalluban commented 3 months ago

Fixed in v9.0.0

Qsppl commented 3 months ago

Thank you