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.

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

tryReference and isValidReference #1087

Merged
merged 4 commits into from
Nov 25, 2018
Merged

Conversation

xaviergonz
Copy link
Contributor

@xaviergonz xaviergonz commented Nov 21, 2018

Basically lately there have been lots of issues about references failing when they were accessed and the referenced node was no longer available, so these two methods give an easier way to use them in those cases.

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.
 *
 * @export
 * @template N
 * @param {(() => N | null | undefined)} getter Function to access the reference.
 * @param {boolean} [checkIfAlive=true] true to also make sure the referenced node is alive (default), false to skip this check.
 * @returns {(N | undefined)}
 */
export function tryReference<N extends IAnyStateTreeNode>(
    getter: () => N | null | undefined,
    checkIfAlive = true
): N | undefined

example:
const ref = tryReference(() => model.someRef);
if (ref) { use it } else {don't use it }

isValidReference

/**
 * Tests if a reference is valid (pointing to an existing node and optionally if alive) and returns if the check passes or not.
 *
 * @export
 * @template N
 * @param {(() => N | null | undefined)} getter Function to access the reference.
 * @param {boolean} [checkIfAlive=true] true to also make sure the referenced node is alive (default), false to skip this check.
 * @returns {boolean}
 */
export function isValidReference<N extends IAnyStateTreeNode>(
    getter: () => N | null | undefined,
    checkIfAlive = true
): boolean

example:
const refIsOk = isValidReference(() => model.someRef);
if (refIsOk) { use it } else {don't use it }

@xaviergonz
Copy link
Contributor Author

xaviergonz commented Nov 21, 2018

This also allows people to write reactions that will auto-purge invalid refs in an easier way

const Todo = ... with some id

const TodoStore = types.model({
  todos: types.array(Todo),
  selectedTodo: types.maybe(types.reference(Todo))
})
.actions(self => ({
  afterCreate() {
    // unselect a Todo when no longer available
    addDisposer(self, reaction(() => isValidReference(() => self.selectedTodo), (valid) => {
      if (!valid) self.selectedTodo = undefined;
    });
  }
}))

Also, since that's such a common pattern it makes me wonder if there should be a types.safeReference(Todo) that automatically turns into undefined when the reference it is pointing to is no longer valid

@xaviergonz
Copy link
Contributor Author

@k-g-a @mweststrate thoughts?

@xaviergonz xaviergonz mentioned this pull request Nov 21, 2018
2 tasks
@fabiosussetto
Copy link

In my opinion this use case is indeed worth a types.safeReference so that it's not necessary to manually deal with the disposer function. For what is worth, I feel like handling references to deleting nodes is one of the missing pieces in this amazing library.

Copy link
Member

@k-g-a k-g-a left a comment

Choose a reason for hiding this comment

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

Looks good to me as it saves some boilerplae. For now we are using almost same checks in our codebase.

@k-g-a
Copy link
Member

k-g-a commented Nov 22, 2018

safeReference seems fine to cope with single references in one row. I'd love to have it.
Do I understand it correctly, that this won't help much for the types.array(type.reference(MyType)) case, as removed/dead references would turn into undefined array items which will lead to "ReferenceError: can not read property X of undefined"?

@xaviergonz
Copy link
Contributor Author

xaviergonz commented Nov 22, 2018

@k-g-a that's right, with types.array(types.safeReference(Todo))array items would turn into undefined
for those we would need something like a types.safeReferenceArray(Todo), that would remove dead refs from the array automatically, and likewise types.safeReferenceMap(Todo)

@xaviergonz
Copy link
Contributor Author

Moved the discussion for types.safeReference and friends to #1088

@xaviergonz xaviergonz merged commit f62fe87 into master Nov 25, 2018
@xaviergonz xaviergonz deleted the isValidReference-getSafeReference branch December 29, 2018 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants