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

safeReference causes "Computed values are not allowed to cause side effects" error #1291

Open
3 tasks done
terrysahaidak opened this issue May 18, 2019 · 3 comments
Open
3 tasks done
Labels
bug Confirmed bug

Comments

@terrysahaidak
Copy link

terrysahaidak commented May 18, 2019

Bug report

  • I've checked documentation and searched for existing issues
  • I've made sure my project is based on the latest MST version
  • Fork this code sandbox or another minimal reproduction.

Sandbox link or minimal reproduction code

https://codesandbox.io/s/mst-safereference-with-computed-bug-7qizx

Describe the expected behavior

I'm building a pretty complex application with a lot of models and computed properties (minimal example of usage in codesandbox). Some of the models require safeReference to be used. But it doesn't work.

Currently, as a workaround, I'm using raw value of rootStore.list instead of rootStore.list.asArray workaround for arrays (not sure if I still need it, do I?), but even if there is no need for asArray for arrays I'm not sure it won't break someday in another way while using several nested views.

Describe the observed behavior

When I've tried to use several nested computed (views) with safeReference in one of my models, got an error (see codesandbox for details):

Error: [mobx] Computed values are not allowed to cause side effects by changing observables that are already being observed. Tried to modify: [email protected]
@terrysahaidak
Copy link
Author

I've played with it and got a version where I haven't used safeReference but it still showed me the same error, so maybe it's not really related. Can't reproduce it again though.

@xaviergonz
Copy link
Contributor

The issue seems to be related to lazy initialization.
slice() in theory shouldn't do any changes in the model, but actually it is accessing all the items in the list and therefore lazy initializing them, and since it is done inside a getter it throws that.

That being said it should be an exception to that error, therefore I think it's a bug

@xaviergonz xaviergonz added the bug Confirmed bug label May 20, 2019
@terrysahaidak
Copy link
Author

My workaround for this is:

export function safeReference(T) {
  return t.reference(T, {
    get(identifier, parent) {
      return resolveIdentifier(T, parent, identifier);
    },
    set(value) {
      return value;
    },
  });
}

It simply returns undefined for unresolved references.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug
Projects
None yet
Development

No branches or pull requests

2 participants