From a06a266f55adc61e3cc27c1aa58f23da3111a10e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Luba=C5=84ski?= Date: Mon, 4 Dec 2023 09:47:56 +0100 Subject: [PATCH] fix(store): unique instance of the draft without an id for each component (#222) --- docs/store/usage.md | 6 ++-- src/store.js | 83 +++++++++++++++++++++------------------------ test/spec/store.js | 11 +++++- 3 files changed, 50 insertions(+), 50 deletions(-) diff --git a/docs/store/usage.md b/docs/store/usage.md index d5e19614..c3ea0855 100644 --- a/docs/store/usage.md +++ b/docs/store/usage.md @@ -307,8 +307,6 @@ define({ ### Enumerables and Listings -For the enumerable or listing model definition, the property creates a read-only connection to the model instance: - #### Explicit Identifier If the `id` option is set, it can be a property name or a function returning the identifier: @@ -358,9 +356,9 @@ el.user = someUser; html``; ``` -#### Cache +#### Rendering in-between state -The significant difference between using the `store.get()` method directly and the factory for properties, which set the `id` option is a unique behavior for returning the last resolved value when identifier changes. The get method always returns the data according to the current state of the model. However, if the `id` option is set for enumerable or listing definition, the factory caches the last value of the property, until the next value is ready. In the meantime, guards return a state of the next value. It means, that it returns the old value, but in the pending state, rather than an empty placeholder for the new value: +The significant difference between using the `store.get()` method directly and the factory, which have the `id` option is a unique behavior for returning the last resolved value when identifier changes. The get method always returns the data according to the current state of the model. However, if the `id` option is set for enumerable or listing definition, the factory caches the last value of the property, until the next value is ready. In the meantime, guards return a state of the next value. It means, that it returns the old value, but in the pending state, rather than an empty placeholder for the new value: ```javascript import { User } from "./models.js"; diff --git a/src/store.js b/src/store.js index 20bdd3a2..7fadd268 100644 --- a/src/store.js +++ b/src/store.js @@ -1493,6 +1493,13 @@ function store(Model, options = {}) { ); } + const resolveId = options.id + ? options.id + : (host, value) => { + if (value !== null && typeof value === "object") return value.id; + return value ? String(value) : undefined; + }; + let draft; if (options.draft) { if (config.list) { @@ -1516,69 +1523,55 @@ function store(Model, options = {}) { draftMap.set(draft, config); Model = draft.model; - } - if (!options.id && config.enumerable && !config.list) { return { get(host, value) { - const valueConfig = definitions.get(value); - const id = valueConfig !== undefined ? value.id : value; + let id = resolveId(host, value); + + if (!id && (value === undefined || value === null)) { + const draftModel = draft.create({}); + id = draftModel.id; - if (draft && (value === undefined || value === null)) { - const draftModel = draft.create({}, { id: undefined }); - syncCache(draft, undefined, draftModel, false); - return get(Model, undefined); + syncCache(draft, draftModel.id, draftModel, false); } - return value ? get(Model, id) : undefined; + return get(Model, id); }, - set: (_, v) => v, - connect: draft + set: options.id ? undefined : (_, v) => v, + connect: config.enumerable ? (host, key) => () => { const model = host[key]; - if (model && model.id) clear(model, true); + if (model) clear(model, true); } : undefined, }; } - return { - get: (host, value) => { - const valueConfig = definitions.get(value); - const id = - (options.id && options.id(host)) || - (valueConfig !== undefined ? value.id : value); - - if (draft && !id && (value === undefined || value === null)) { - const draftModel = draft.create({}); - syncCache(draft, undefined, draftModel, false); - return get(Model, undefined); - } - - if (!config.list && config.enumerable && id === undefined) - return undefined; + if (options.id) { + return { + get(host, value) { + const id = resolveId(host); + const nextValue = config.list || id ? get(Model, id) : undefined; - const nextValue = get(Model, id); + if (nextValue !== value && ready(value) && !ready(nextValue)) { + const tempValue = cloneModel(value); + cache.set(tempValue, "state", () => getModelState(nextValue)); + return tempValue; + } - if (nextValue !== value && ready(value) && !ready(nextValue)) { - const tempValue = cloneModel(value); - cache.set(tempValue, "state", () => getModelState(nextValue)); - return tempValue; - } + return nextValue; + }, + }; + } - return nextValue; + return { + get(host, value) { + const id = resolveId(host, value); + return !config.enumerable || config.list || value + ? get(Model, id) + : undefined; }, - set: - (!options.id && config.list) || (draft && !config.enumerable) - ? (_, v) => v - : undefined, - connect: - draft && config.enumerable - ? (host, key) => () => { - const model = host[key]; - if (model && model.id) clear(model, true); - } - : undefined, + set: (_, v) => v, }; } diff --git a/test/spec/store.js b/test/spec/store.js index 418cbce8..58c4d0bd 100644 --- a/test/spec/store.js +++ b/test/spec/store.js @@ -1326,9 +1326,18 @@ describe("store:", () => { }); describe("in draft mode", () => { - it("returns new model instance for not initialized model", () => { + it("returns new model instance for not initialized model for each component", () => { expect(el.draftwithoutid).toBeDefined(); expect(store.ready(el.draftwithoutid)).toBe(true); + + const el2 = document.createElement("test-store-factory-enumerable"); + document.body.appendChild(el2); + + expect(el2.draftwithoutid).toBeDefined(); + expect(el2.draftwithoutid).not.toBe(el.draftwithoutid); + expect(store.ready(el2.draftwithoutid)).toBe(true); + + document.body.removeChild(el2); }); it("updates not initialized draft new model instance", () =>