Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added possibility to respect observable decorator variants, sets can only be observed shallowly #313

Closed
wants to merge 2 commits into from
Closed

Added possibility to respect observable decorator variants, sets can only be observed shallowly #313

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d40444e
Added possibility to respect observable decorator variants, sets can …
TomasChmelik Jan 6, 2023
d9a128b
Fixed deep observation of objects added later, refactoring
TomasChmelik Jan 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 54 additions & 23 deletions src/deepObserve.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ import {
IMapDidChange,
values,
entries,
isObservableSet,
IArraySplice,
_getAdministration
} from "mobx"
import { IDisposer } from "./utils"

type IChange = IObjectDidChange | IArrayDidChange | IMapDidChange
type IChange = IObjectDidChange | IArrayDidChange | IMapDidChange | IArraySplice

type Entry = {
dispose: IDisposer
Expand All @@ -30,7 +33,7 @@ function buildPath(entry: Entry | undefined): string {
}

function isRecursivelyObservable(thing: any) {
return isObservableObject(thing) || isObservableArray(thing) || isObservableMap(thing)
return isObservableObject(thing) || isObservableArray(thing) || isObservableMap(thing) || isObservableSet(thing)
}

/**
Expand All @@ -53,39 +56,43 @@ function isRecursivelyObservable(thing: any) {
*/
export function deepObserve<T = any>(
target: T,
listener: (change: IChange, path: string, root: T) => void
listener: (change: IChange, path: string, root: T) => void,
respectAnnotations = false
): IDisposer {
const entrySet = new WeakMap<any, Entry>()

function genericListener(change: IChange) {
function shallowListener(change: IChange): void {
const entry = entrySet.get(change.object)!
listener(change, buildPath(entry), target)
}

function deepListener(change: IChange): void {
const entry = entrySet.get(change.object)!
processChange(change, entry)
listener(change, buildPath(entry), target)
}

function processChange(change: IChange, parent: Entry) {
function processChange(change: IChange, changedObjectEntry: Entry): void {
switch (change.type) {
// Object changes
case "add": // also for map
observeRecursively(change.newValue, parent, change.name)
case "add": // map or object
observeKey(change.object, change.name, change.newValue, changedObjectEntry)
break
case "update": // also for array and map
case "update": // array, map or object
unobserveRecursively(change.oldValue)
observeRecursively(
change.newValue,
parent,
(change as any).name || "" + (change as any).index
)
if (change.observableKind === "array") {
observeKey(change.object, "" + change.index, change.newValue, changedObjectEntry)
} else {
observeKey(change.object, change.name, change.newValue, changedObjectEntry)
}
break
case "remove": // object
case "delete": // map
unobserveRecursively(change.oldValue)
break
// Array changes
case "splice":
case "splice": // array
change.removed.map(unobserveRecursively)
change.added.forEach((value, idx) =>
observeRecursively(value, parent, "" + (change.index + idx))
change.added.forEach((value: any, idx: any) =>
observeKey(change.object, "" + (change.index + idx), value, changedObjectEntry)
)
// update paths
for (let i = change.index + change.addedCount; i < change.object.length; i++) {
Expand All @@ -98,7 +105,10 @@ export function deepObserve<T = any>(
}
}

function observeRecursively(thing: any, parent: Entry | undefined, path: string) {
function observeRecursively(thing: any, parent: Entry | undefined, path: string, deep: boolean): void {
// sets can't be observed deeply
deep = deep && !isObservableSet(thing)

if (isRecursivelyObservable(thing)) {
const entry = entrySet.get(thing)
if (entry) {
Expand All @@ -115,15 +125,36 @@ export function deepObserve<T = any>(
const entry = {
parent,
path,
dispose: observe(thing, genericListener),
dispose: observe(thing, deep ? deepListener : shallowListener)
}
entrySet.set(thing, entry)
entries(thing).forEach(([key, value]) => observeRecursively(value, entry, key))

if (deep) {
entries(thing).forEach(([key, value]) => observeKey(thing, key, value, entry))
}
}
}
}

function getKeyAnnotationType(thing: any, key: string): string | undefined {
return _getAdministration(thing)?.appliedAnnotations_?.[key]?.annotationType_
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

appliedAnnotations_ is private, mangled and actually not even available in production build:
https://github.com/mobxjs/mobx/blob/78d1aa2362b4dc5d521518688d6ac7e2d4f7ad3a/packages/mobx/src/types/observableobject.ts#L114-L117

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know if there is a way to get to the annotations in production build? Or is this idea just not possible?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that you've misunderstood how ref and shallow works. They say how a non-observable value (plain object/array/set/map) should be converted into observable - if the value is already observable, these decorators have no effect. So if you have an observable Entity it remains observable even if you assign it to ref prop or push to shallow array. Since these don't remove observability, it would be weird if deepObserve would ignore it.

You could additionally wrap the Entity into something actually non-observable to stop the recursion:

@observable.ref _entity = { ref: entity }

get entity() {
  return this._entity.ref;
}

set entity(entity) {
  this._entity = { ref: entity }
}

get to the annotations in production build

I think you can get the enhancer via getAtom(thing, prop?).enhancer, but to find out what kind of enhancer it is, you actually have to call it and check the result, like this:
#170 (comment)

Copy link
Author

@TomasChmelik TomasChmelik Feb 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that's exactly what we want. We have entities which reference each other (decorated with observable.ref) and each one have their own observable properties. We want to observe the entities in a way that we get info only when their own property changes, not property of some other entity then reference.

class EntityA {
    @observable.ref entityB?: EntityB
    @observable propertyA: any
    @observable somethingDeep: string[] = []

    constructor() {
        makeObservable(this)
    }
}

class EntityB {
    @observable propertyB: string

    constructor() {
        makeObservable(this)
    }
}

const entityA = new EntityA()
const entityB = new EntityB()
entityA.entityB = entityB

const dispose = deepObserve(entityA, console.log)

entityA.propertyA = 'something' // I want this to be logged
entityA.somethingDeep.push('something') // I want this to be logged
entityB.propertyB = 'something' // I don't want this to be logged

dispose()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case you should use observe, not deepObserve. As @urugator noted ref/shallow modifiers are completely unrelated to deep resp shallow observing. Simply put, deepObserve is a wrapper around observe, that sets ups new observe listeners recursively, until no observables could be reached anymore. So if an an observable.ref points to an observable, it will recurse into it. MobX-state-tree has a semantic distinction between 'owning' an object, and loosely referring to it, but in MobX we make no such distinction.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But with observe I wouldn't get the update when doing entityA.somethingDeep.push('something'). Anyway it looks like it is not possible to do this in the production build or you just don't want this to be included.

Closing PR

}

function observeKey(thing: any, key: string, value: any, thingEntry: Entry): void {
if (respectAnnotations) {
switch (getKeyAnnotationType(thing, key)) {
case "observable.ref":
return
case "observable.shallow":
observeRecursively(value, thingEntry, key, false)
return
}
}

observeRecursively(value, thingEntry, key, true)
}

function unobserveRecursively(thing: any) {
function unobserveRecursively(thing: any): void {
if (isRecursivelyObservable(thing)) {
const entry = entrySet.get(thing)
if (!entry) return
Expand All @@ -133,7 +164,7 @@ export function deepObserve<T = any>(
}
}

observeRecursively(target, undefined, "")
observeRecursively(target, undefined, "", true)

return () => {
unobserveRecursively(target)
Expand Down