MobX documentation encourages circular dependency between stores

[NikolajLeischner]

The MobX documentation describes a pattern of having a "root store", which has references to every other store, and injecting it into every store: https://mobx.js.org/defining-data-stores.html

This is effectively a circular dependency between all stores, which is commonly seen as an anti-pattern.

Two immediate negative effects of this are:

• Worse testability - every unit test of a store needs a mock of the root store.
• Because every store can access every other store, developers that aren't careful end up writing spaghetti code.

Why recommend this, instead of explicitly injecting the dependencies a store actually needs?

[kubk]

Completely agree, I also always pass dependencies explicitly to avoid Service Locator anti-pattern. Dependency Injection doesn't hide dependencies (unlike Service Locator) making code more testable and straightforward.

[danielkcz]

I haven't really read that recommendation, but generally, I would say it's more approachable to newcomers to have a single state. Especially if they are converting from Redux. The mentioned "negative effects" are sort of opinionated and manageable in overall.

Either way, it would be most welcome if you are willing to write up an alternative approach that feels easier for you.

[NikolajLeischner]

I've actually had discussions with people coming from Redux, who cited that specific recommendation from the documentation as off-putting and as an argument for why MobX in their mind was no good alternative to Redux.

I'd be happy to provide a suggestion, I'll write something up within the next few days :)

[mweststrate]

MobX does technically zero care about how you organise your store (unlike Redux). The documented pattern is just one of dozens approaches that could be taken in organising models / store / views / data. It was written because "we don't care" was too much freedom for many to handle. So the documentation is just a way in which it could be done, and you should definitely not read too much into it. If you have any argument to deviate from it, by all means do.

DI can be great, but others find it confusing. But most importantly, if we would use a DI container, it would immediately be read as 'mobx does need a DI container'. Like our current docs are sometimes already read as: MobX needs a single root container. That all isn't the case; mobx is just about propagating state changes from A to B. How that state is organized is just as irrelevant to MobX as in which folders you put your components.

So please don't read the docs as 'this is the best way to architect your application', but rather 'if you have no clue how to get started, here is a way'. That is what we tried to say in the intro:

This section contains some of the best practices for building large scale maintainable projects we discovered at Mendix while working with MobX. This section is opinionated and you are in no way forced to apply these practices. There are many ways of working with MobX and React, and this is just one of them.

If that can all be clarified better, feel free to PR on the docs

[ghost]

There could be a "Defining data stores (DI) {🚀}" doc to show the DI approach, so the reader can learn about both and comes to his own conclusion. I'm interested in what you'll write up, and am ready to go over it and correct any grammar mistakes to match the docs.

And what Michel wrote above should be incorporated into the existing doc as well, making it crystal clear that you can go whichever way you want to.

(if it's agreed to have both approaches documented, only thinking out loud)

Edit: and if this is the main reason Redux users are reluctant to try MobX out or switch over, then this should be docs priority since it'd fix the current X amount of people going "head first into a wall" to a clean judo move using their own momentum.

[danielkcz]

@zangornjak The important idea with V6 was to have a unified and ideally one way on how to do things. Once we start introducing X different variants on how to manage stores, it will become a mess again and people will be confused.

As Michel said, MobX is not about this at all. It's up to the user to decide what suits him/her. Besides, some people will probably soon post various blog posts on these topics with their opinions. We can link to these posts as alternatives, but it's probably not a good idea to have several different approaches within the docs.

[ghost]

I understand, only MobX core approaches and keep anything that could be opinionated out of the docs. The current Defining data stores is there just for you to see an opinion.

The reason for my "positive" response was the Edit part, because that's where I see the actual value.

[danielkcz]

Well, this isn't some sort of business or rivalry competition. If Redux users are satisfied, they don't need to switch. We don't need to convince them that much they should switch right now. MobX already its own community that's growing and that's totally fine.

It's a world full of assumptions. Someone might like that because it's similar to Redux and they wouldn't need to change that much in their mindset. Other people will dislike it because of true hate toward Redux. You can't really satisfy everyone.

[ghost]

Got it, I'll stop bike shedding with you at this point.

[imjuni]

I think this problem some of simple. In javascript case you can use non import. But this problem is seriously in TypeScript.

I concern about this problem. So, I found my solution. See below code.

// file Todo.ts
interface Todo {
  title: string;
  content: string;
}

// file RootStore.ts
import './TodoStore.ts';
import './UserStore.ts';

class RootStore {
    public userStore: UserStore;
    public todoStore: TodoStore;
    
    constructor() {
        this.userStore = new UserStore(this)
        this.todoStore = new TodoStore(this)
    }
}

// file UserStore.ts
class UserStore {
    public rootStore: { rootStore: TodoStore };

    constructor(rootStore) {
        this.rootStore = rootStore
    }

    getTodos(user) {
        // Access todoStore through the root store.
        return this.rootStore.todoStore.todos.filter(todo => todo.author === user)
    }
}

// file TodoStore.ts
import './Todo.ts';

class TodoStore {
    public todos: Todo[] = [];
    public rootStore: { rootStore: TodoStore };

    constructor(rootStore: { rootStore: TodoStore }) {
        makeAutoObservable(this, { rootStore: false })
        this.rootStore = rootStore
    }
}

Don't use concrete type RootStore. Use predicatable type. This solution can't resolve construction order problem. And you don't initialize any member value in constructor. But this solution don't raise a circular dependency problem.

[mweststrate]

You can use `import type {Todo} from "./Todo` to avoid circular deps (note the type keyword).

…

On Thu, Mar 25, 2021 at 10:01 AM ByungJoon Lee ***@***.***> wrote: I think this problem some of simple. In javascript case you can use non import. But this problem is seriously in TypeScript. I concern about this problem. So, I found my solution. See below code. // file Todo.tsinterface Todo { title: string; content: string;} // file RootStore.tsimport './TodoStore.ts';import './UserStore.ts'; class RootStore { public userStore: UserStore; public todoStore: TodoStore; constructor() { this.userStore = new UserStore(this) this.todoStore = new TodoStore(this) }} // file UserStore.tsclass UserStore { public rootStore: { rootStore: TodoStore }; constructor(rootStore) { this.rootStore = rootStore } getTodos(user) { // Access todoStore through the root store. return this.rootStore.todoStore.todos.filter(todo => todo.author === user) }} // file TodoStore.tsimport './Todo.ts'; class TodoStore { public todos: Todo[] = []; public rootStore: { rootStore: TodoStore }; constructor(rootStore: { rootStore: TodoStore }) { makeAutoObservable(this, { rootStore: false }) this.rootStore = rootStore }} Don't use concrete type RootStore. Use predicatable type. This solution can't resolve construction order problem. And you don't initialize any member value in constructor. But this solution don't raise a circular dependency problem. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2483 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBEIZBJ3HPLESYRB2KDTFMCXZANCNFSM4SEMR5HA> .

[imjuni]

@mweststrate Thank you!