-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add data facade utility #15
Conversation
1. Define your interface and create handlers | ||
2. Create facade context and export `FacadeProvider` and `useFacade` | ||
3. Wrap your application with `FacadeProvider` | ||
4. Use `useFacade` hook to access the facade in your components |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanna expand on this, but figured I'd keep it simple while I get feedback on the implementation.
import { AccountsMethods } from "./types"; | ||
|
||
const accounts = ["alice", "bob", "carol"]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am expecting that we have a handlers file per datasource? ie /accounts/mock.ts, /accounts/walletServer.ts, etc?
import { createFacadeContext } from "data-facade"; | ||
import { accountsHandlers } from "./accounts/handlers"; | ||
|
||
const facadeContext = createFacadeContext(accountsHandlers); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is importing the mock implementation, how will this look when we have multiple handlers for different systems? Will there be a conditional here based on a setting? I am imagining that a user might change data backend at any point. Will the backend change result in a account reset/chain reset? Will that have any impact on your representation here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one major comment about the different providers. Would be nice to have that sorted, but I don't think it is totally critical to do in this particular PR.
Adds a
data-facade
package so that we can define the API interface and its implementation in isolation from the React app, so that we can swap out data sources in the future while reducing the need to refactor the React components themselves.