Open xaviergonz opened 5 years ago
Initial reaction: like! Although it is kinda set it does leak the normalization abstraction.
Just a random idea, didn't think it through, what if we would keep things as is, but that asRef(m.ref)
would return the object you described above? In that case people can always opt in to the more expicit api, but there is still only one reference type. (asRef(m.ref)
would require a weird temporarily global flag, like with auto value unboxing, but I think it does read better than asRef(m, "ref")
?
What about asRef(() => m.ref)
if we deref an invalid ref directly (when passing it as a direct arg) then it would throw before getting to the asRef
function
The one thing I don't quite like though is that the typing would be still kind of hard to do...
Well, now that I think about it, it is not supported at all now, now you need to do m.ref = 5 as any
, whereas with this would be done with
asRef(() => m.ref).id = 5
So something like this then?
asRef(() => m.ref): {
readonly valid: boolean, // true if the ref is not broken, false if it is broken
id: number | string // get or set id
// not sure if the following ones would be needed, but I guess they would be cool if you want to pass the ref object around
deref(): T,
safeDeref(): T | undefined,
setTarget(T)
// alternatively
target: T, // deref + setTarget
readonly safeTarget: T | undefined // alternative to safeDeref()
}
The other thing I don't like is that you need to make sure to use the parent object when accessing the child or else it won't work, whereas asRef(m, "ref")
wouldn't require an arrow function and would make this clearer, so it would be harder to mess the call up (and can be typed ok in TS), so I think that might be a better option actually.
Hey, what about external (not-from-tree) refs?
If this fixes #1284, it would dramatically simplify some complex models I have.
Are we currently awaiting a decision, or for someone to volunteer to implement?
I think it would be great if @xaviergonz suggestion was implemented:
asRef(owner, propertyName) => {
readonly valid: boolean, // true if the ref is not broken, false if it is broken
readonly id: number | string // get
deref(): T,
safeDeref(): T | undefined,
setTarget(T)
setId(number | string)
}
@Amareis those can already be implemented using custom refs.
While impelmenting stuff described in #1355 I thought of adding special types referenceArray
/referenceMap
.
Just a basic API overview:
const myArrayType = types.array(SomeSubType); // it would be impossible to inline this into model
const modelType = types.model({
items: myArrayType
itemReferences: types.arrayReference(myArrayType , mode: 'standalone' | 'safe' | 'sync', custom?: ReferenceOptionsGetSet)
})
The mode
parameter will enforce one of the following behaviours:
1) 'standalone' is basically the same as type.array(type.reference(SubType))
- no custom add/delete handling
2) 'safe' is basically the same as type.array(type.safeReference(SubType))
- safely handle delete from source, do not handle add
3) 'sync' is as 'safe', but with add operation support
None of the variants assume 'two-way-sync' (i.e. adding to itemReferences
does not modify the original, it just breaks if there is no match in items
), but this could be simply implemented via custom.set
.
The main benefit is that we could remove fair amount of logic brought by safeReference
(internal hooks, tracking id nodes and so on).
New implementation birdview:
1) we introduce two internal storages (let's name it sources
and referrers
for now) - one per tree, same as with IdentifiersCache
2) array/map types get additional methods: registerReference
/unregisterReference
3) every array/map type registeres itself within corresponding sources
storage upon tree creation and checks if there are any referrers
4) every referenceArray/referenceMap type registers itself within corresponding referrers
storage upon creation and checks if there are any sources
5) when steps 3 or 4 hit the match, corresponding source
's registerReference
is called with referrer
;
6) didChange
(or maybe even willChange
?) handlers of array/map type begin to respect registered referrer
's and alter them
7) upon referrer
destruction we unregister it from source
8) upon source
destruction we clear all referrer
's
Steps 3 and 4 are needed, because any part of the tree could be 'hidden' inside optional
and will appear later on, durning interaction with the tree.
To support custom references across trees types.arrayReference
/types.mapReference
could accept instance getter delegate:
const myArrayType = types.array(SomeSubType); // it would be impossible to inline this into model
const sourceModelType = types.model({ items: myArrayType })
const sourceModel = modelType.create(...)
const referrerModelType = types.model({
refs: types.arrayReference(() => sourceModel.items, 'sync')
})
const referrerModel = referrerModelType.create();
referrerModel.refs // is fully syncronized with sourceModel.items
Even simpler API: types.arrayReference((self) => self.items, 'sync')
- by having self
we can always reference own tree's parts, but are free to get anything we want (as the last example in my previous comment), even types.arrayReference((self) => getEnv(self).someOtherInjectedStore.items, 'sync')
for those who prefer DI over direct imports
I really love the idea @k-g-a, especially having designated types for collections of references could greatly simplify stuff.
I think synced deletes should be the default (and non configurable ?)
Overal I think we might have designed references too transparent, I think in future improvements we should maintain explicit reference objects with an explicit api, so that we can introduce all the utility methods discussed above and also introduce lazy resolving / loading of references.
What prevents us to implement references as an external library? All the things around creating and accessing the global registry of models of a type? Maybe MST can expose these lower level apis And let developers have their own flavour, and give the maintainers more freedom with breaking changes.
I'm currently working on a similar implementation to the proposal by @xaviergonz.
It's based on a real model. Why do I need a real model? Because then I can use getParent
on that and get the actual parent of the reference.
For example, here is the case.
We have PostModel which has a reference to ApplicationModel. That application is going to be stored in some Entities
model since many models can reference it.
const ApplicationModel = types.model("Application", {
id: types.identifierNumber
});
const PostModel = types.model("Post", {
id: types.identifierNumber,
application: types.reference(ApplicationModel)
});
const EntitiesModel = types.model("EntitiesModel", {
applications: types.map(ApplicationModel),
posts: types.map(PostModel)
});
const RootModel = types.model("Root", {
posts: types.array(types.reference(PostModel)),
entities: types.optional(EntitiesModel, {})
});
So now we can create our root
:
const root = RootModel.create({
posts: [1],
entities: {
posts: {
1: {
id: 1,
application: 1
}
},
applications: {
1: {
id: 1
}
}
}
});
The whole thing works great with the current reference implementation. But there is one thing which gets tricky.
Since Application can basically be a part of the post, we need some access to it from the application, too.
But when we do getParent(self)
in the Application model, we get Map as a parent because the map is a parent of the model, but the post is a parent of the reference to that Application model.
The only implementation I could think about - to use a real model and custom registry for all the references to some type.
// first of all we need some place to store all our references
const RefsRegistry = new Map<IAnyComplexType, Map<string, IStateTreeNode>>();
export function createEntityRef<T extends IAnyComplexType>(
entityName: string,
Model: T
) {
// find the registry based on the model type
let currentRegistry = RefsRegistry.get(Model);
if (!currentRegistry) {
currentRegistry = new Map();
RefsRegistry.set(Model, currentRegistry);
}
const refModel = types
.model(`${Model.name}Ref`, {
// or can be a number
id: types.number
})
.volatile(self => ({
// we need internal ref to store our reference
_refId: `${Model.name}Ref-${uuid()}`
}))
.views((self: any) => ({
get current() {
const entities = getRoot<any>(self).entities;
const entityModel = entities[entityName];
// currently I resolve it from the entityModel but it can be stored anywhere
return entityModel.get(self.id) as Instance<T>;
}
}))
.actions(self => ({
// we store our reference as soon as this model created
afterCreate() {
currentRegistry!.set(self._refId, self);
}
}));
return refModel;
}
With that fabric now we can replace our types.reference(ApplicationModel)
with our custom ref model:
const applicationRef = createEntityRef('applications', ApplicationModel);
const PostModel = types.model("Post", {
id: types.identifierNumber,
application: applicationRef
});
In order to make it work out of the box, we need to add snapshotProcessor to the ref model so it can receive number or string as a snapshot and convert it to { id: snapshot }
.
Now we can use this function inside of ApplicationModel to get the parent of type Post of the ref (or parents):
export function getReferenceParentOfType<T extends IAnyModelType>(
model: IStateTreeNode,
parentType: T
): T["Type"] | undefined {
const type = getType(model);
const registry = RefsRegistry.get(type);
if (!registry) {
return undefined;
}
for (let value of registry.values()) {
try {
return getParentOfType<T>(value, parentType);
} catch {}
}
return undefined;
}
And still, this works great. But there is another problem - mobx-state-tree is lazy by default. That means this block will be executed only when I actually access this ref.
afterCreate() {
currentRegistry!.set(self._refId, self);
},
This is probably OK for most of the cases, but there is another cool one.
The other really useful case is to be able to find all the references to the specific model and be able to work with those references (for example, to remove from all the lists):
export function resolveModelReferences<T extends IAnyModelType>(
type: T
): undefined | any[] {
const registry = RefsRegistry.get(type);
if (!registry) {
return undefined;
}
return [...registry.values()];
}
But this will resolve all the accessed reference models because rest of them won't be stored inside the registry of the specific model type so again this block won't be executed until I access the reference model:
afterCreate() {
currentRegistry!.set(self._refId, self);
},
Any ideas on how to work around this?
Right now refs are kind of hard to grasp for users, since they are actually ids that hide a resolution logic to objects.
Also they exhibit the following problems:
The proposal is a new API for using refs, something like this:
Technically it could be a non breaking change by adding a new ref type (or actually two with safeReference), but that'd mean we'd end up with 4 possible ref types
Maybe something like types.ref / types.safeRef
Or maybe something best left for a major version?