From 5488410c0c5cf18ff974d182bc9a31e35725c4a9 Mon Sep 17 00:00:00 2001 From: Harry Brundage Date: Thu, 17 Oct 2024 14:58:18 +0000 Subject: [PATCH] Add support for acceptsUndefined: false to safe references MST proper supports an `acceptsUndefined: false` on safe references that automatically prunes invalid references from parent maps and arrays. I wanna use it in Gadget but noticed that it no worky -- we pass the option through to observable instances fine but didn't do anything with it in readonly instances. This adds handling for the special case, which requires arrays and maps to look at the child type, and if it is a safe reference with the option set, omit unresolved entries from the result. Woop woop. --- package.json | 5 +- spec/reference.spec.ts | 316 +++++++++++++++++++++++++++++---------- src/array.ts | 8 +- src/fast-instantiator.ts | 20 ++- src/map.ts | 7 + src/reference.ts | 13 +- 6 files changed, 269 insertions(+), 100 deletions(-) diff --git a/package.json b/package.json index 2692e4c..7d0bca3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@gadgetinc/mobx-quick-tree", - "version": "0.7.7", + "version": "0.7.8", "description": "A mirror of mobx-state-tree's API to construct fast, read-only instances that share all the same views", "source": "src/index.ts", "main": "dist/src/index.js", @@ -65,8 +65,5 @@ "ts-node": "^10.9.2", "typescript": "^5.4.5", "yargs": "^17.7.2" - }, - "volta": { - "node": "18.12.1" } } diff --git a/spec/reference.spec.ts b/spec/reference.spec.ts index 9caed9f..d7f6deb 100644 --- a/spec/reference.spec.ts +++ b/spec/reference.spec.ts @@ -4,6 +4,7 @@ import type { IAnyClassModelType, Instance, SnapshotOrInstance } from "../src"; import { ClassModel, register, types } from "../src"; import { TestClassModel } from "./fixtures/TestClassModel"; import { TestModel, TestModelSnapshot } from "./fixtures/TestModel"; +import { create } from "./helpers"; const Referrable = types.model("Referenced", { key: types.identifier, @@ -32,107 +33,256 @@ const Root = types.model("Reference Model", { }); describe("references", () => { - test("can resolve valid references", () => { - const root = Root.createReadOnly({ - model: { - ref: "item-a", - }, - refs: [ - { key: "item-a", count: 12 }, - { key: "item-b", count: 523 }, - ], + describe.each([ + ["read-only", true], + ["observable", false], + ])("%s", (_name, readOnly) => { + test("can resolve valid references", () => { + const root = create( + Root, + { + model: { + ref: "item-a", + }, + refs: [ + { key: "item-a", count: 12 }, + { key: "item-b", count: 523 }, + ], + }, + readOnly, + ); + + expect(root.model.ref).toEqual( + expect.objectContaining({ + key: "item-a", + count: 12, + }), + ); }); - expect(root.model.ref).toEqual( - expect.objectContaining({ - key: "item-a", - count: 12, - }), - ); - }); + test("can resolve valid safe references", () => { + const root = create( + Root, + { + model: { + ref: "item-a", + safeRef: "item-b", + }, + refs: [ + { key: "item-a", count: 12 }, + { key: "item-b", count: 523 }, + ], + }, + readOnly, + ); + + expect(root.model.safeRef).toEqual( + expect.objectContaining({ + key: "item-b", + count: 523, + }), + ); + }); - test("throws for invalid refs", () => { - const createRoot = () => - Root.createReadOnly({ - model: { - ref: "item-c", + test("does not throw for invalid safe references", () => { + const root = create( + Root, + { + model: { + ref: "item-a", + safeRef: "item-c", + }, + refs: [ + { key: "item-a", count: 12 }, + { key: "item-b", count: 523 }, + ], }, - refs: [ - { key: "item-a", count: 12 }, - { key: "item-b", count: 523 }, - ], + readOnly, + ); + + expect(root.model.safeRef).toBeUndefined(); + }); + + test("safe references marked with allowUndefined false are non-nullable in types-style arrays", () => { + const Referencer = types.model("Referencer", { + safeRefs: types.array(types.safeReference(Referrable, { acceptsUndefined: false })), }); - expect(createRoot).toThrow(); - }); + const Root = types.model("Reference Model", { + refs: types.array(Referrable), + model: Referencer, + }); + const root = create( + Root, + { + refs: [ + { key: "item-a", count: 12 }, + { key: "item-b", count: 523 }, + ], + model: { + safeRefs: ["item-a", "item-c"], + }, + }, + readOnly, + ); - test("can resolve valid safe references", () => { - const root = Root.createReadOnly({ - model: { - ref: "item-a", - safeRef: "item-b", - }, - refs: [ - { key: "item-a", count: 12 }, - { key: "item-b", count: 523 }, - ], + expect(root.model.safeRefs.map((obj) => obj.key)).toEqual(["item-a"]); + + type instanceType = (typeof root.model.safeRefs)[0]; + assert>(false); + assert>(false); }); - expect(root.model.safeRef).toEqual( - expect.objectContaining({ - key: "item-b", - count: 523, - }), - ); - }); + test("safe references marked with allowUndefined false are non-nullable in types-style maps", () => { + const Referencer = types.model("Referencer", { + safeRefs: types.map(types.safeReference(Referrable, { acceptsUndefined: false })), + }); + + const Root = types.model("Reference Model", { + refs: types.array(Referrable), + model: Referencer, + }); + const root = create( + Root, + { + refs: [ + { key: "item-a", count: 12 }, + { key: "item-b", count: 523 }, + ], + model: { + safeRefs: { + "item-a": "item-a", + "item-c": "item-c", + }, + }, + }, + readOnly, + ); - test("does not throw for invalid safe references", () => { - const root = Root.createReadOnly({ - model: { - ref: "item-a", - safeRef: "item-c", - }, - refs: [ - { key: "item-a", count: 12 }, - { key: "item-b", count: 523 }, - ], + expect([...root.model.safeRefs.keys()]).toEqual(["item-a"]); }); - expect(root.model.safeRef).toBeUndefined(); - }); + test("safe references marked with allowUndefined false are non-nullable in class model arrays", () => { + @register + class Referencer extends ClassModel({ + safeRefs: types.array(types.safeReference(Referrable, { acceptsUndefined: false })), + }) {} + + @register + class Root extends ClassModel({ + refs: types.array(Referrable), + model: Referencer, + }) {} - test("references are equal to the instances they refer to", () => { - const root = Root.createReadOnly({ - model: { - ref: "item-a", - safeRef: "item-b", - }, - refs: [ - { key: "item-a", count: 12 }, - { key: "item-b", count: 523 }, - ], + const root = create( + Root, + { + refs: [ + { key: "item-a", count: 12 }, + { key: "item-b", count: 523 }, + ], + model: { + safeRefs: ["item-a", "item-c"], + }, + }, + readOnly, + ); + + expect(root.model.safeRefs.map((obj) => obj.key)).toEqual(["item-a"]); + + type instanceType = (typeof root.model.safeRefs)[0]; + assert>(false); + assert>(false); }); - expect(root.model.ref).toBe(root.refs[0]); - expect(root.model.ref).toEqual(root.refs[0]); - expect(root.model.ref).toStrictEqual(root.refs[0]); - }); + test("safe references marked with allowUndefined false are non-nullable in class model maps", () => { + @register + class Referencer extends ClassModel({ + safeRefs: types.map(types.safeReference(Referrable, { acceptsUndefined: false })), + }) {} + + @register + class Root extends ClassModel({ + refs: types.array(Referrable), + model: Referencer, + }) {} - test("safe references are equal to the instances they refer to", () => { - const root = Root.createReadOnly({ - model: { - ref: "item-a", - safeRef: "item-b", - }, - refs: [ - { key: "item-a", count: 12 }, - { key: "item-b", count: 523 }, - ], + const root = create( + Root, + { + refs: [ + { key: "item-a", count: 12 }, + { key: "item-b", count: 523 }, + ], + model: { + safeRefs: { + "item-a": "item-a", + "item-c": "item-c", + }, + }, + }, + readOnly, + ); + + expect([...root.model.safeRefs.keys()]).toEqual(["item-a"]); + }); + + test("references are equal to the instances they refer to", () => { + const root = create( + Root, + { + model: { + ref: "item-a", + safeRef: "item-b", + }, + refs: [ + { key: "item-a", count: 12 }, + { key: "item-b", count: 523 }, + ], + }, + readOnly, + ); + + expect(root.model.ref).toBe(root.refs[0]); + expect(root.model.ref).toEqual(root.refs[0]); + expect(root.model.ref).toStrictEqual(root.refs[0]); }); - expect(root.model.safeRef).toBe(root.refs[1]); - expect(root.model.safeRef).toEqual(root.refs[1]); - expect(root.model.safeRef).toStrictEqual(root.refs[1]); + test("safe references are equal to the instances they refer to", () => { + const root = create( + Root, + { + model: { + ref: "item-a", + safeRef: "item-b", + }, + refs: [ + { key: "item-a", count: 12 }, + { key: "item-b", count: 523 }, + ], + }, + readOnly, + ); + + expect(root.model.safeRef).toBe(root.refs[1]); + expect(root.model.safeRef).toEqual(root.refs[1]); + expect(root.model.safeRef).toStrictEqual(root.refs[1]); + }); + }); + + test("throws for invalid refs", () => { + const createRoot = () => + Root.createReadOnly({ + model: { + ref: "item-c", + }, + refs: [ + { key: "item-a", count: 12 }, + { key: "item-b", count: 523 }, + ], + }); + + expect(createRoot).toThrow(); }); test("instances of a model reference are assignable to instances of the model", () => { diff --git a/src/array.ts b/src/array.ts index 450b308..fd979fe 100644 --- a/src/array.ts +++ b/src/array.ts @@ -4,6 +4,7 @@ import { ensureRegistered } from "./class-model"; import { getSnapshot } from "./snapshot"; import { $context, $parent, $readOnly, $type } from "./symbols"; import type { IAnyStateTreeNode, IAnyType, IArrayType, IMSTArray, IStateTreeNode, Instance, TreeContext } from "./types"; +import { SafeReferenceType } from "./reference"; export class QuickArray extends Array> implements IMSTArray { static get [Symbol.species]() { @@ -82,8 +83,13 @@ export class ArrayType extends BaseType(this, parent, context); if (snapshot) { - array.push(...snapshot.map((element) => this.childrenType.instantiate(element, context, array))); + let instances = snapshot.map((element) => this.childrenType.instantiate(element, context, array)); + if (this.childrenType instanceof SafeReferenceType && this.childrenType.options?.acceptsUndefined === false) { + instances = instances.filter((instance) => instance != null); + } + array.push(...instances); } + return array as this["InstanceType"]; } diff --git a/src/fast-instantiator.ts b/src/fast-instantiator.ts index 4a45d32..3b7810e 100644 --- a/src/fast-instantiator.ts +++ b/src/fast-instantiator.ts @@ -311,23 +311,27 @@ export class InstantiatorBuilder): string { + private assignmentExpressionForMapType(key: string, type: MapType): string { const mapVarName = `map${key}`; const snapshotVarName = `snapshotValue${key}`; + const removeUndefineds = + type.childrenType instanceof SafeReferenceType && type.childrenType.options?.acceptsUndefined === false + ? "if (item == null) { continue; }" + : ""; + return ` const ${mapVarName} = new QuickMap(${this.alias(`model.properties["${key}"]`)}, this, context); this["${key}"] = ${mapVarName}; const ${snapshotVarName} = snapshot?.["${key}"]; if (${snapshotVarName}) { for (const key in ${snapshotVarName}) { - ${mapVarName}.set( - key, - ${this.alias(`model.properties["${key}"].childrenType`)}.instantiate( - ${snapshotVarName}[key], - context, - ${mapVarName} - ) + const item = ${this.alias(`model.properties["${key}"].childrenType`)}.instantiate( + ${snapshotVarName}[key], + context, + ${mapVarName} ); + ${removeUndefineds} + ${mapVarName}.set(key, item); } }`; } diff --git a/src/map.ts b/src/map.ts index 3973011..4a5ffcc 100644 --- a/src/map.ts +++ b/src/map.ts @@ -15,6 +15,7 @@ import type { TreeContext, SnapshotOut, } from "./types"; +import { SafeReferenceType } from "./reference"; export class QuickMap extends Map> implements IMSTMap { static get [Symbol.species]() { @@ -112,6 +113,12 @@ export class MapType extends BaseType< if (snapshot) { for (const key in snapshot) { const item = this.childrenType.instantiate(snapshot[key], context, map); + if (this.childrenType instanceof SafeReferenceType && this.childrenType.options?.acceptsUndefined === false) { + if (item == null) { + continue; + } + } + map.set(key, item); } } diff --git a/src/reference.ts b/src/reference.ts index fbf043b..a29d8fc 100644 --- a/src/reference.ts +++ b/src/reference.ts @@ -55,7 +55,7 @@ export class SafeReferenceType extends BaseT > { constructor( readonly targetType: IAnyComplexType, - options?: SafeReferenceOptions, + readonly options?: SafeReferenceOptions, ) { super(types.safeReference(targetType.mstType, options)); } @@ -85,10 +85,15 @@ export const reference = ( return new ReferenceType(targetType, options); }; -export const safeReference = ( +export function safeReference( + subType: IT, + options: SafeReferenceOptions & { acceptsUndefined: false }, +): IReferenceType; +export function safeReference(subType: IT, options?: SafeReferenceOptions): IMaybeType>; +export function safeReference( targetType: TargetType, options?: SafeReferenceOptions, -): IMaybeType> => { +): IMaybeType> { ensureRegistered(targetType); return new SafeReferenceType(targetType, options); -}; +}