Functions that create circular references need to have their return type explicitly typed

[Amareis]

Bug report

• [X] I've checked documentation and searched for existing issues
• [ ] I've made sure my project is based on the latest MST version I cannot updates to latest version because of this bug
• [X] Fork this code sandbox or another minimal reproduction.

Sandbox link or minimal reproduction code https://codesandbox.io/s/mj33l002zp

Describe the expected behavior chats view of User model should have Chat[] return type, as in sandbox.

Describe the observed behavior After updating mobx-state-tree version to 3.8.0 and later, chats view on User model will have implicit any return type. I think it's because of #1074 and that's really annoying bug, whick blocks updating to newest versions.

[xaviergonz]

 get chats() { // <-- [ts] 'chats' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions. [7023]
            return store.chats.filter(
                chat => !!chat.users.find(u => u.id === self.id),
            )
        },

if you add a type annotation to it

get chats(): Instance<typeof Chat>[] `

it works

Basically typescript is complaining that it cannot infer the type due to circular references store -> Store -> Store.users -> User -> User.chats -> store.chats -> Chat -> Chat.users -> User -> back to User.chats... etc

[Amareis]

But why it works in 3.7.1 and early?

[xaviergonz]

probably because some types became broken when the types were made faster up until that version and they were not properly detected, but just a guess

[xaviergonz]

Btw, on a totally unrelated note I'd change return store.chats.filter( to return getRoot<typeof Store>(self).chats.filter( so it is not tightly coupled with a given store instance :)

[Amareis]

Yep, in sandbox code just reduced to minimal, but in actual code getRoot is used.

Can more smart typing be restored in some way? If this is impossible, issue can be closed, I thinks.

[Amareis]

Oh, that's explicit return type didn't help much because of circular dependency with tones of types. Temporarily solve it with any, because i really needs to update mst, but it's defeinitely not the best solution in my life.

[xaviergonz]

I'll take a look to see if I can figure a way so that's not needed, but no promises

[xaviergonz]

I tried, it is related to the Omit now used in here

export type RedefineIStateTreeNode<T, STN extends IAnyStateTreeNode> = T extends IAnyStateTreeNode
    ? Omit<T, typeof $stateTreeNodeTypes> & STN
    : T

but it can't be fixed without restructuring how the whole IStateTreeNode thing works (which is no small feat) :( Basically IStateTreeNode would need to be separated from "T" part of types and then moved to somewhere else

[tomdid]

I would love to get this fixed too as for now instead of using getRoot(self).id / getParent(self).id , I am forced to pass the id as an argument to action, which is not that nice.

[xaviergonz]

Or you can add the return type explicitly like I commented above

[poalrom]

Its not working with getParentOfType: https://codesandbox.io/s/2p1qn9n1op

Get error: 'Post' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.

[xaviergonz]

That's a different issue (Post depends on Category, and Category depends on Post). In that particular case do NOT explicitly set the return type and let it be inferred and it should work

const Post = types
  .model({
    text: types.string
  })
  .views(self => ({
    getCategory() { // do NOT add a return type here
      return getParentOfType(self, Category);
    }
  }))
  .actions(self => ({
    getCategoryTitle() {
      return self.getCategory().title;
    }
  }));

const Category = types.model({
  title: types.string,
  post: types.array(Post)
});

[poalrom]

When I dont set return type its not working with async functions: https://codesandbox.io/s/yplz3lkv7z

Should I open another issue for this?

[AndrewSorokin]

Hi!

Hi guys! Would it help to resolve circular dependency issues? https://github.com/microsoft/TypeScript/pull/33050

[mweststrate]

Afaik it doesn't cover all / complex cases. But feel free to upgrade your TS version and give it a spint :)

On Wed, Dec 11, 2019 at 10:43 AM AndrewSorokin notifications@github.com wrote:

Hi!

Hi guys! Would it help to resolve circular dependency issues? microsoft/TypeScript#33050 https://github.com/microsoft/TypeScript/pull/33050

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx-state-tree/issues/1157?email_source=notifications&email_token=AAN4NBEN5SSPV2GGDQ3S3FDQYC74VA5CNFSM4GTCHBUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGSVOHI#issuecomment-564483869, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBBI6OGYCMOBJVACHMTQYC74VANCNFSM4GTCHBUA .

[thegedge]

I think we'll likely close this issue as "won't fix" 😞

It's challenging to bend TS to our will here because of how inference and checking are done at the same time (see see https://github.com/microsoft/TypeScript/issues/26623 for some light discussion on that). Generally the only way around this is:

1. Use classes, or
2. explicit return type annotations.

For (1), there are a couple of class-y MST implementations floating around, such as https://github.com/charto/classy-mst and https://github.com/gadget-inc/mobx-quick-tree/. Otherwise, your best bet is (2), as has already been discussed in this thread :)

[thegedge]

~Actually, scrap that. Although I don't understand why, I think I may have a solution here.~

Nevermind. I wrote an incorrect type that just happened to typecheck 😢

[coolsoftwaretyler]

@thegedge - before we close this out, I think we should consider updating the docs with some of the tips and tricks in this thread, along with what you've just posted. I actually point people here for this comment in particular once in a while. I think once issues get closed, they tend to be harder to discover.

I'm going to tag this with docs and say the thing we need to do is help people find the alternative approaches.

I'll assign this to myself, but mostly will work off your great wrap up answer here.

[coolsoftwaretyler]

Oh whoops, I don't think that answer works. See: https://codesandbox.io/p/sandbox/circular-typing-example-jf3q73?file=%2Fsrc%2Findex.ts%3A8%2C16

I have hit this in my own work, and I haven't found a workaround. I think you're right that we're limited by TypeScript here, and class-based implementations might work.

My other recommendation for folks trying to use getParentOfType is to use it from outside the model definitions. So instead of including it as part of the definition in a view or action of a model, you might be able to use it from the outside of the tree like this:

import { getParentOfType, t } from "mobx-state-tree";

const Post = t.model({
  text: t.string,
});

const Category = t.model({
  title: t.string,
  post: t.array(Post),
});

const post = Post.create({ text: "Hello" });
const category = Category.create({
  title: "Category Title",
  post: [post],
});

const parentCategory = getParentOfType(post, Category);

console.log(parentCategory.title);

You can see that working here: https://codesandbox.io/p/sandbox/get-parent-of-type-from-elsewhere-3mlk8k?file=%2Fsrc%2Findex.ts%3A1%2C1-21%2C1

I know it doesn't always work, and often you want to traverse parents from a particular node. But in some cases, accessing parents from outside the tree may help as a stopgap measure.

While I agree we can't fix this, I think we should keep the issue open as a known bug and consider it for future updates. It would be a lot of work, but it would be great if we figured this out in a future redesign or something.