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

Best practises for singleton stores in ES6 #1192

Closed
LeonardoGentile opened this issue Oct 6, 2017 · 4 comments
Closed

Best practises for singleton stores in ES6 #1192

LeonardoGentile opened this issue Oct 6, 2017 · 4 comments

Comments

@LeonardoGentile
Copy link

This is not strictly a mobx question, but it is more about the best practise to create singleton stores.
In your documentation you states:

Make sure this state is a singleton.

This can be achieved in many different ways, an example and a commonly used pattern (also used in many of your examples) is to export a class instance from a module, this will become a singleton because of module caching:

class UiState {
    @observable language = "en_US";
    @observable count = 0;
   
    extraMoney = null;

    constructor() {
        this.extraMoney = 34;
    }

    @computed getRealCount () {
        return this.count + 99 
    }
}

singleton = new UiState();
export default singleton;

Some says this is a bad pattern because it relies on module caching and because it would still be possible to create new instances of the classes if the consumer really wants it, for example:

import singleton from './UiState';
const newStoreInstance = new singleton.constructor();

Others also says we should never use a class instance for creating a singleton, singleton should be just plain objects. Even tough I agree that mobx stores could be just plain objects that makes a bit more difficult to organise code compared to the class syntax. So another common pattern I've seen for creating singleton from classes is this:

/* 
 * Setting up block level variable to store class state
 */
let instance = null;

class Store{  
    constructor() {
        if(!instance){
              instance = this;
        }
        return instance;
    }
}

There are also other ways of creating singletons, here I have only presented 2 just for example.

Also to avoid circular dependencies it might be a good idea to never export single instances from modules, we should export class/object definitions and then instantiate/initialise them in the entry point.

What's your take on this, what would you consider to be the best practise to create singleton stores in ES6 for a complex application?

@iamdanthedev
Copy link

iamdanthedev commented Oct 8, 2017

Hi
IMHO, the pattern upstairs takes away control over when the Store is created. Also from olden days of c# development I remember they always proposed to keep in singleton instance in a static field of a class, which is quite handy. That's my template for singleton stores below. The getStore() is not needed in principle when <Provider /> from mobx-react is used

export class Store {

  public static INSTANCE: Store;

  constructor() {

    if (Store.INSTANCE) {
      throw new Error('Store is a singleton');
    }

    Store.INSTANCE = this;
  }

}

export function createStore(): Store {
  return new Store();
}

export function getStore(): Store {
  return Store.INSTANCE;
}

@urugator
Copy link
Collaborator

urugator commented Oct 8, 2017

because it would still be possible to create new instances of the classes if the consumer really wants it

Doesn't matter. The consumer can't make any assumption about something.constructor - whether it's even defined and what it actually does (the consumer doesn't know how something was created).

So another common pattern I've seen for creating singleton from classes is this:

Note it still relies on module cache. Imho, it's worse than exporting the object itself, because it hides the same functionallity behind a syntax, which suggests absolutely different behavior (creation of a new object).

Others also says we should never use a class instance for creating a singleton, singleton should be just plain objects.

I think the reasoning here is that it doesn't make sense to have a class(prototype) for something, which is never actually created more than once. Aside from possible confusion, I don't see any practical implications...

we should export class/object definitions and then instantiate/initialise them in the entry point.

This. People should forget about singletons and rather focus on how to manage dependencies in general.

@nighca
Copy link

nighca commented Oct 10, 2017

Agrees with @urugator.

Discussion here may helps :) @LeonardoGentile

@mweststrate
Copy link
Member

@LeonardoGentile I changed the docs, no clue why I put singleton in there, as I am generally quite oppossed to them, and the rest of the document kinda correctly suggests to avoid singletons. Main problem with singletons is that they cannot be reset, which can be an issue for 1) routing, 2) state rest during tests 3) paralellization during tests

Beyond that, I think this discussions goes a bit beyond mobx itself and might better fit SO

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

No branches or pull requests

5 participants