Skip to content

Commit f62fe87

Browse files
authored
tryReference and isValidReference (#1087)
* implementation * renamed to tryReference * updated changelog * check that reaction can be used to clear invalid references
1 parent c2a409b commit f62fe87

File tree

8 files changed

+636
-411
lines changed

8 files changed

+636
-411
lines changed

API.md

+445-403
Large diffs are not rendered by default.

README.md

+2
Original file line numberDiff line numberDiff line change
@@ -1019,6 +1019,7 @@ See the [full API docs](API.md) for more details.
10191019
| [`isAlive(node)`](API.md#isalive) | Returns `true` if `node` is alive |
10201020
| [`isStateTreeNode(value)`](API.md#isstatetreenode) | Returns `true` if `value` is a node of a mobx-state-tree |
10211021
| [`isProtected(value)`](API.md#isprotected) | Returns `true` if the given node is protected, see [actions](#actions) |
1022+
| [`isValidReference(() => node | null | undefined, checkIfAlive = true)`](API.md#isvalidreference) | Tests if a reference is valid (pointing to an existing node and optionally if alive) and returns if the check passes or not. |
10221023
| [`isRoot(node)`](API.md#isroot) | Returns true if `node` has no parents |
10231024
| [`joinJsonPath(parts)`](API.md#joinjsonpath) | Joins and escapes the given path `parts` into a JSON path |
10241025
| [`onAction(node, (actionDescription) => void)`](API.md#onaction) | A built-in middleware that calls the provided callback with an action description upon each invocation. Returns disposer |
@@ -1037,6 +1038,7 @@ See the [full API docs](API.md) for more details.
10371038
| [`splitJsonPath(path)`](API.md#splitjsonpath) | Splits and unescapes the given JSON `path` into path parts |
10381039
| [`typecheck(type, value)`](API.md#typecheck) | Typechecks a value against a type. Throws on errors. Use this if you need typechecks even in a production build. |
10391040
| [`tryResolve(node, path)`](API.md#tryresolve) | Like `resolve`, but just returns `null` if resolving fails at any point in the path |
1041+
| [`tryReference(() => node | null | undefined, checkIfAlive = true)`](API.md#tryreference) | Tests if a reference is valid (pointing to an existing node and optionally if alive) and returns such reference if it the check passes, else it returns undefined. |
10401042
| [`unprotect(node)`](API.md#unprotect) | Unprotects `node`, making it possible to directly modify any value in the subtree, without actions |
10411043
| [`walk(startNode, (node) => void)`](API.md#walk) | Performs a depth-first walk through a tree |
10421044
| [`escapeJsonPath(path)`](API.md#escapejsonpath) | escape special characters in an identifier, according to http://tools.ietf.org/html/rfc6901 |

changelog.md

+2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
- Added `tryReference` and `isValidReference` to use references that might be no longer pointing to any nodes in a safe way through [#1087](https://github.com/mobxjs/mobx-state-tree/pull/1087) by [@xaviergonz](https://github.com/xaviergonz)
2+
13
# 3.8.1
24

35
- Fixed non-initialized nodes not being destroyed [#1080](https://github.com/mobxjs/mobx-state-tree/issues/1080) through [#1082](https://github.com/mobxjs/mobx-state-tree/pull/1082) by [@k-g-a](https://github.com/k-g-a)

packages/mobx-state-tree/__tests__/core/api.test.ts

+2
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ const METHODS_AND_INTERNAL_TYPES = stringToArray(`
7070
isReferenceType,
7171
isRefinementType,
7272
isUnionType,
73+
isValidReference,
74+
tryReference,
7375
7476
types
7577
`)

packages/mobx-state-tree/__tests__/core/reference.test.ts

+96-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@ import {
1616
SnapshotOrInstance,
1717
isAlive,
1818
destroy,
19-
castToReferenceSnapshot
19+
castToReferenceSnapshot,
20+
tryReference,
21+
isValidReference,
22+
isStateTreeNode,
23+
addDisposer
2024
} from "../../src"
2125

2226
test("it should support prefixed paths in maps", () => {
@@ -48,6 +52,7 @@ test("it should support prefixed paths in maps", () => {
4852
users: { "17": { id: "17", name: "Michel" }, "18": { id: "18", name: "Noa" } }
4953
} as SnapshotOut<typeof store>)
5054
})
55+
5156
test("it should support prefixed paths in arrays", () => {
5257
const User = types.model({
5358
id: types.identifier,
@@ -74,6 +79,7 @@ test("it should support prefixed paths in arrays", () => {
7479
users: [{ id: "17", name: "Michel" }, { id: "18", name: "Noa" }]
7580
} as SnapshotOut<typeof store>)
7681
})
82+
7783
if (process.env.NODE_ENV !== "production") {
7884
test("identifiers are required", () => {
7985
const Todo = types.model({
@@ -85,6 +91,7 @@ if (process.env.NODE_ENV !== "production") {
8591
" `undefined` is not assignable to type: `identifier` (Value is not a valid identifier, expected a string)"
8692
)
8793
})
94+
8895
test("identifiers cannot be modified", () => {
8996
const Todo = types.model({
9097
id: types.identifier
@@ -99,6 +106,7 @@ if (process.env.NODE_ENV !== "production") {
99106
)
100107
})
101108
}
109+
102110
test("it should resolve refs during creation, when using path", () => {
103111
const values: number[] = []
104112
const Book = types.model({
@@ -132,6 +140,7 @@ test("it should resolve refs during creation, when using path", () => {
132140
expect(s.entries.reduce((a, e) => a + e.price, 0)).toBe(8)
133141
expect(values).toEqual([4, 8])
134142
})
143+
135144
test("it should resolve refs over late types", () => {
136145
const Book = types.model({
137146
id: types.identifier,
@@ -158,6 +167,7 @@ test("it should resolve refs over late types", () => {
158167
expect(s.entries[0].price).toBe(4)
159168
expect(s.entries.reduce((a, e) => a + e.price, 0)).toBe(4)
160169
})
170+
161171
test("it should resolve refs during creation, when using generic reference", () => {
162172
const values: number[] = []
163173
const Book = types.model({
@@ -223,6 +233,7 @@ test("string identifiers should not accept numbers", () => {
223233
expect(F2.is({ id: "4" })).toBe(true)
224234
expect(F2.is({ id: 4 })).toBe(false)
225235
})
236+
226237
test("122 - identifiers should support numbers as well", () => {
227238
const F = types.model({
228239
id: types.identifierNumber
@@ -237,6 +248,7 @@ test("122 - identifiers should support numbers as well", () => {
237248
expect(F.is({ id: "4" })).toBe(false)
238249
expect(F.is({ id: "bla" })).toBe(false)
239250
})
251+
240252
test("self reference with a late type", () => {
241253
const Book = types.model("Book", {
242254
id: types.identifier,
@@ -266,6 +278,7 @@ test("self reference with a late type", () => {
266278
s.addBook(book2)
267279
expect((s.books[1].reference as Instance<typeof Book>).genre).toBe("thriller")
268280
})
281+
269282
test("when applying a snapshot, reference should resolve correctly if value added after", () => {
270283
const Box = types.model({
271284
id: types.identifierNumber,
@@ -282,6 +295,7 @@ test("when applying a snapshot, reference should resolve correctly if value adde
282295
})
283296
).not.toThrow()
284297
})
298+
285299
test("it should fail when reference snapshot is ambiguous", () => {
286300
const Box = types.model("Box", {
287301
id: types.identifierNumber,
@@ -324,6 +338,7 @@ test("it should fail when reference snapshot is ambiguous", () => {
324338
"[mobx-state-tree] Cannot resolve a reference to type '(Box | Arrow)' with id: '1' unambigously, there are multiple candidates: /boxes/0, /arrows/1"
325339
)
326340
})
341+
327342
test("it should support array of references", () => {
328343
const Box = types.model({
329344
id: types.identifierNumber,
@@ -347,6 +362,7 @@ test("it should support array of references", () => {
347362
}).not.toThrow()
348363
expect(getSnapshot(store.selected)).toEqual([1, 2])
349364
})
365+
350366
test("it should restore array of references from snapshot", () => {
351367
const Box = types.model({
352368
id: types.identifierNumber,
@@ -364,6 +380,7 @@ test("it should restore array of references from snapshot", () => {
364380
expect(store.selected[0] === store.boxes[0]).toEqual(true)
365381
expect(store.selected[1] === store.boxes[1]).toEqual(true)
366382
})
383+
367384
test("it should support map of references", () => {
368385
const Box = types.model({
369386
id: types.identifierNumber,
@@ -387,6 +404,7 @@ test("it should support map of references", () => {
387404
}).not.toThrow()
388405
expect(getSnapshot(store.selected)).toEqual({ from: 1, to: 2 })
389406
})
407+
390408
test("it should restore map of references from snapshot", () => {
391409
const Box = types.model({
392410
id: types.identifierNumber,
@@ -404,6 +422,7 @@ test("it should restore map of references from snapshot", () => {
404422
expect(store.selected.get("from") === store.boxes[0]).toEqual(true)
405423
expect(store.selected.get("to") === store.boxes[1]).toEqual(true)
406424
})
425+
407426
test("it should support relative lookups", () => {
408427
const Node = types.model({
409428
id: types.identifierNumber,
@@ -447,6 +466,7 @@ test("it should support relative lookups", () => {
447466
expect(resolveIdentifier(Node, n5, 4)).toBe(n2.children[0])
448467
expect(resolveIdentifier(Node, n2.children[0], 5)).toBe(n5)
449468
})
469+
450470
test("References are non-nullable by default", () => {
451471
const Todo = types.model({
452472
id: types.identifierNumber
@@ -489,6 +509,7 @@ test("References are non-nullable by default", () => {
489509
expect(() => ((store as any).ref = undefined)).toThrow(/Error while converting/)
490510
}
491511
})
512+
492513
test("References are described properly", () => {
493514
const Todo = types.model({
494515
id: types.identifierNumber
@@ -502,6 +523,7 @@ test("References are described properly", () => {
502523
"{ todo: ({ id: identifierNumber } | undefined?); ref: reference(AnonymousModel); maybeRef: (reference(AnonymousModel) | undefined?) }"
503524
)
504525
})
526+
505527
test("References in recursive structures", () => {
506528
const Folder = types.model("Folder", {
507529
id: types.identifierNumber,
@@ -590,6 +612,7 @@ test("References in recursive structures", () => {
590612
expect(store.objects.get("1")).toBe(store.tree.children[0].data)
591613
expect(store.objects.get("2")).toBe(store.tree.children[0].children[0].data)
592614
})
615+
593616
test("it should applyPatch references in array", () => {
594617
const Item = types.model("Item", {
595618
id: types.identifier,
@@ -648,6 +671,7 @@ test("it should applyPatch references in array", () => {
648671
hovers: []
649672
})
650673
})
674+
651675
test("it should applySnapshot references in array", () => {
652676
const Item = types.model("Item", {
653677
id: types.identifier,
@@ -850,7 +874,7 @@ test("#1052 - Reference returns destroyed model after subtree replacing", () =>
850874
}
851875
})
852876

853-
it("#1080 - does not crash trying to resolve a reference to a destroyed+recreated model", () => {
877+
test("#1080 - does not crash trying to resolve a reference to a destroyed+recreated model", () => {
854878
const Branch = types.model("Branch", {
855879
id: types.identifierNumber,
856880
name: types.string
@@ -928,3 +952,73 @@ it("#1080 - does not crash trying to resolve a reference to a destroyed+recreate
928952
name: "Branch 2"
929953
})
930954
})
955+
956+
test("tryReference / isValidReference", () => {
957+
const Todo = types.model({ id: types.identifier })
958+
959+
const TodoStore = types
960+
.model({
961+
todos: types.array(Todo),
962+
ref1: types.maybe(types.reference(Todo)),
963+
ref2: types.maybeNull(types.reference(Todo)),
964+
ref3: types.maybe(types.reference(Todo))
965+
})
966+
.actions(self => ({
967+
clearRef3() {
968+
self.ref3 = undefined
969+
},
970+
afterCreate() {
971+
addDisposer(
972+
self,
973+
reaction(
974+
() => isValidReference(() => self.ref3),
975+
valid => {
976+
if (!valid) {
977+
this.clearRef3()
978+
}
979+
},
980+
{ fireImmediately: true }
981+
)
982+
)
983+
}
984+
}))
985+
986+
const store = TodoStore.create({
987+
todos: [{ id: "1" }, { id: "2" }, { id: "3" }]
988+
})
989+
990+
expect(tryReference(() => store.ref1)).toBeUndefined()
991+
expect(tryReference(() => store.ref2)).toBeUndefined()
992+
expect(isValidReference(() => store.ref1)).toBe(false)
993+
expect(isValidReference(() => store.ref2)).toBe(false)
994+
995+
unprotect(store)
996+
store.ref1 = store.todos[0]
997+
store.ref2 = store.todos[1]
998+
store.ref3 = store.todos[2]
999+
1000+
expect(isStateTreeNode(store.ref1)).toBe(true)
1001+
expect(isStateTreeNode(store.ref2)).toBe(true)
1002+
1003+
expect(tryReference(() => store.ref1)).toBeDefined()
1004+
expect(tryReference(() => store.ref2)).toBeDefined()
1005+
expect(isValidReference(() => store.ref1)).toBe(true)
1006+
expect(isValidReference(() => store.ref2)).toBe(true)
1007+
1008+
store.todos = cast([])
1009+
1010+
expect(tryReference(() => store.ref1)).toBeUndefined()
1011+
expect(tryReference(() => store.ref2)).toBeUndefined()
1012+
expect(isValidReference(() => store.ref1)).toBe(false)
1013+
expect(isValidReference(() => store.ref2)).toBe(false)
1014+
1015+
// the reaction should have triggered and set this to undefined
1016+
expect(store.ref3).toBe(undefined)
1017+
1018+
expect(() => tryReference(() => 5 as any)).toThrowError(
1019+
"The reference to be checked is not one of node, null or undefined"
1020+
)
1021+
expect(() => isValidReference(() => 5 as any)).toThrowError(
1022+
"The reference to be checked is not one of node, null or undefined"
1023+
)
1024+
})

packages/mobx-state-tree/src/core/mst-operations.ts

+67-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ import {
2323
isModelType,
2424
INode,
2525
ModelPrimitive,
26-
ExtractNodeC
26+
ExtractNodeC,
27+
InvalidReferenceError
2728
} from "../internal"
2829

2930
export type TypeOrStateTreeNodeToStateTreeNode<
@@ -555,6 +556,71 @@ export function getIdentifier(target: IAnyStateTreeNode): string | null {
555556
return getStateTreeNode(target).identifier
556557
}
557558

559+
/**
560+
* Tests if a reference is valid (pointing to an existing node and optionally if alive) and returns such reference if it the check passes,
561+
* else it returns undefined.
562+
*
563+
* @export
564+
* @template N
565+
* @param {(() => N | null | undefined)} getter Function to access the reference.
566+
* @param {boolean} [checkIfAlive=true] true to also make sure the referenced node is alive (default), false to skip this check.
567+
* @returns {(N | undefined)}
568+
*/
569+
export function tryReference<N extends IAnyStateTreeNode>(
570+
getter: () => N | null | undefined,
571+
checkIfAlive = true
572+
): N | undefined {
573+
try {
574+
const node = getter()
575+
if (node === undefined || node === null) {
576+
return undefined
577+
} else if (isStateTreeNode(node)) {
578+
if (!checkIfAlive) {
579+
return node
580+
} else {
581+
return isAlive(node) ? node : undefined
582+
}
583+
} else {
584+
return fail("The reference to be checked is not one of node, null or undefined")
585+
}
586+
} catch (e) {
587+
if (e instanceof InvalidReferenceError) {
588+
return undefined
589+
}
590+
throw e
591+
}
592+
}
593+
594+
/**
595+
* Tests if a reference is valid (pointing to an existing node and optionally if alive) and returns if the check passes or not.
596+
*
597+
* @export
598+
* @template N
599+
* @param {(() => N | null | undefined)} getter Function to access the reference.
600+
* @param {boolean} [checkIfAlive=true] true to also make sure the referenced node is alive (default), false to skip this check.
601+
* @returns {boolean}
602+
*/
603+
export function isValidReference<N extends IAnyStateTreeNode>(
604+
getter: () => N | null | undefined,
605+
checkIfAlive = true
606+
): boolean {
607+
try {
608+
const node = getter()
609+
if (node === undefined || node === null) {
610+
return false
611+
} else if (isStateTreeNode(node)) {
612+
return checkIfAlive ? isAlive(node) : true
613+
} else {
614+
return fail("The reference to be checked is not one of node, null or undefined")
615+
}
616+
} catch (e) {
617+
if (e instanceof InvalidReferenceError) {
618+
return false
619+
}
620+
throw e
621+
}
622+
}
623+
558624
/**
559625
*
560626
*

packages/mobx-state-tree/src/index.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,9 @@ import {
179179
isPrimitiveType,
180180
isReferenceType,
181181
isRefinementType,
182-
isUnionType
182+
isUnionType,
183+
tryReference,
184+
isValidReference
183185
} from "./internal"
184186

185187
export {
@@ -301,5 +303,7 @@ export {
301303
isPrimitiveType,
302304
isReferenceType,
303305
isRefinementType,
304-
isUnionType
306+
isUnionType,
307+
tryReference,
308+
isValidReference
305309
}

0 commit comments

Comments
 (0)