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

added the initialization function of the dataSource called it on the … #146

Open
wants to merge 1 commit into
base: experimental
Choose a base branch
from

Conversation

Amoodaa
Copy link
Collaborator

@Amoodaa Amoodaa commented Oct 3, 2019

…app.componentDidMount and only when the user is logged

relates #137

…app.componentDidMount and only when the user is logged

relates #137
@11janci
Copy link

11janci commented Oct 4, 2019

ok I think I finally understand, you're trying to do a data store for data shared between components, similar to Redux.. Got confused by the dataSource name and the combination with the API 🙂

That's not a bad idea, of course in practice you would use an existing solution like Redux instead of doing your own, but outside of that, this is a nice implementation! 👍
My only suggestion would be to separate the concerns:

  • take the API outside of the store, do not mix it
  • allow arbitrary object for the state (instead of having fixed structure that contains users, channels, etc.)

Doing that would give you a generic store that you can reuse for different use cases and across projects. So instead of for example the init() function where you call some application-specific APIs, there would be just one function to update the data:

update: newState => {
    state = newState; // just an example, you would have to properly merge the objects
    notifySubscribers(state);
}

This function would be called instead of component's setState in existing API calls.

api.getChannels().then(res => dataStore.update({ channels: res.channels }));

And if you want to simplify it, so that you don't have to call the update function everywhere, you can couple the store with the APIs in a separate module

// this will update the store automatically
dataStoreApiWrapper.getChannels();

The problem with this solution is that it calls setState for every subscribed component every time the data store changes, which means that every component will get re-rendered even when its state necessarily didn't change. There are ways to deal with that, but overall, you're on the right path, nice job guys..
Hope this all makes at least a bit of sense 😆

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

Successfully merging this pull request may close these issues.

2 participants