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

State change notifications #31

Open
kirlat opened this issue Jun 22, 2020 · 4 comments
Open

State change notifications #31

kirlat opened this issue Jun 22, 2020 · 4 comments

Comments

@kirlat
Copy link
Member

kirlat commented Jun 22, 2020

Right now we're using two ways to send notification to modules. One is with Vuex and watchers and another one using our own publisher/subscriber events model. We probably should create guidelines on when to prefer one way other the other. Without it, it might get messy.

We probably don't want to depend on Vuex too much: it is an external library that is out of our control. If it will change, we will be forced to rewrite our code, even if we don't to. It it will stop being supported, we have to search for a replacement. But it might make sense to use it whenever we use Vue already: UI components. Vuex works with Vue in many great ways.

So maybe we can use Vuex to notify Vue UI components about changes that would affect the visual state of an application, because that's the area of responsibility of UI components? I.e. when UI needs to know whether to show a certain button, or a tab? The distinction then will be that we use the Vuex store to manage application state that affects the UI components and use PS Events to manage application state that affects business logic.

@balmas, @irina060981: What do you think? Does it make sense?

@balmas
Copy link
Member

balmas commented Jun 22, 2020

I think this division makes sense. I am also thinking that we will need to make the UserDataManager class more robust so that we can use it to more fully manage user state.

@irina060981
Copy link
Member

I believe, that we now

  • use Vuex inside one repo - components
  • use events between different repos

As for Vue components

  • we use direct child/parent relations between Vue components where we don't need to track changes elsewhere in other components
  • we use Vuex mutations when we need track changes in the bunch of components

@kirlat , what exactly changes do you suggest?

@kirlat
Copy link
Member Author

kirlat commented Jun 23, 2020

As for Vue components

  • we use direct child/parent relations between Vue components where we don't need to track changes elsewhere in other components
  • we use Vuex mutations when we need track changes in the bunch of components

That sounds about right to me. I do not suggest to changes anything in the components library right now, I just think it will be good to establish clear rules for the future. Important rule should be that the non-UI modules in components should not use Vuex.

I think we should change existing code only if it does not follow the rules, and only if we're making changes that are caused by other requirements. So even if it needs any improvements, we should do them as we go.

@balmas
Copy link
Member

balmas commented Jun 23, 2020

Agree on all points so far.

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

3 participants