fix(store): factory with id resolving to undefined, draft models with…

…out id
hybridsjs · Oct 15, 2021 · 83bcc1d · 83bcc1d
1 parent d067560
commit 83bcc1d
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 12 deletions.
diff --git a/src/store.js b/src/store.js
@@ -448,7 +448,7 @@ function setupModel(Model, nested) {
       if (key === "id") {
         if (Model[key] !== true) {
           throw TypeError(
-            "The 'id' property in model definition must be set to 'true' or not be defined",
+            "The 'id' property in the model definition must be set to 'true' or not defined",
           );
         }
         return (model, data, lastModel) => {
@@ -874,7 +874,7 @@ function get(Model, id) {
   if (config.enumerable) {
     stringId = stringifyId(id);
 
-    if (!config.list && !stringId) {
+    if (!stringId && !config.list && !draftMap.get(config)) {
       throw TypeError(
         stringifyModel(
           Model,
@@ -1439,30 +1439,32 @@ function store(Model, options = {}) {
     return {
       get(host, value) {
         const valueConfig = definitions.get(value);
-        let id = valueConfig !== undefined ? value.id : value;
+        const id = valueConfig !== undefined ? value.id : value;
 
-        if (!id && options.draft) {
-          const draftModel = options.draft.create({});
-          syncCache(options.draft, draftModel.id, draftModel, false);
-          id = draftModel.id;
+        if (options.draft && (value === undefined || value === null)) {
+          const draftModel = options.draft.create({}, { id: undefined });
+          syncCache(options.draft, undefined, draftModel, false);
+          return get(Model, undefined);
         }
 
-        return id ? get(Model, id) : undefined;
+        return value ? get(Model, id) : undefined;
       },
       set: (_, v) => v,
     };
   }
 
   return {
     get: (host, value) => {
-      let id = (options.id && options.id(host)) || (value && value.id);
+      const id = (options.id && options.id(host)) || (value && value.id);
 
-      if (!id && !value && options.draft) {
+      if (options.draft && !id && (value === undefined || value === null)) {
         const draftModel = options.draft.create({});
-        syncCache(options.draft, draftModel.id, draftModel, false);
-        id = draftModel.id;
+        syncCache(options.draft, undefined, draftModel, false);
+        return get(Model, undefined);
       }
 
+      if (config.enumerable && id === undefined) return undefined;
+
       const nextValue = get(Model, id);
 
       if (nextValue !== value && ready(value) && !ready(nextValue)) {

diff --git a/test/spec/store.js b/test/spec/store.js
@@ -1196,6 +1196,7 @@ describe("store:", () => {
         define({
           tag: "test-store-factory-enumerable",
           modelId: "1",
+          byundefined: store(Model, { id: () => undefined }),
           byprop: store(Model, { id: "modelId" }),
           byfn: store(Model, { id: ({ modelId }) => modelId }),
           withoutid: store(Model),
@@ -1216,6 +1217,10 @@ describe("store:", () => {
           }).toThrow();
         });
 
+        it("returns undefined when id resolves to undefined", () => {
+          expect(el.byundefined).toBe(undefined);
+        });
+
         it("gets and updates store model instance", () => {
           let pendingModel = el.byprop;
           expect(store.pending(pendingModel)).toBeTruthy();