-
Notifications
You must be signed in to change notification settings - Fork 21
Standalone External Stores #25
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
base: master
Are you sure you want to change the base?
Conversation
|
@d3dc This is pretty cool! Yeah it goes way further than i expected. But makes sense to keep the logic inside the store container. As for middleware, do we need it? There's a declarative middleware sort-of construct: https://codesandbox.io/embed/mjv84k1kn9 But maybe for logging, re-winding state, etc. - could be useful. |
|
@drcmda I already use I use the word middleware specifically because of the standard redux use cases of logging and debugging. I can see scenario's where |
|
Old Examples: |
|
@drcmda I went ahead and set this up to not make too much public API. Providers now have a subscription to their store and will re-render according to ordinary If you pass an action as a prop to a Provider, what should happen? Should the Provider provide an action that only updates its internal state? Or should the Provider wrap that action to affect the external store? |
…em in a Provider's local state.
External stores have a `setState` function External stores have `state.setState` by default. External stores have a `wrapActions` function to provide middleware on this side (should this use subscriptions?) The first middleware is always `store.setState`
Remove public setState. Move state creation to its own function.
Overwritten actions are rebound to only affect this Provider
Codecov Report
@@ Coverage Diff @@
## master #25 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 6 6
Lines 98 101 +3
=====================================
+ Hits 98 101 +3
Continue to review full report at Codecov.
|
|
@d3dc hard to say, can you make a small example? Btw, do you need more access? I'd be totally open to share authorship if good ideas find their way into the lib. |
|
@drcmda I'm not sure what else I should be contributing. I detail a lot since I feel like I'm stepping on the toes of a "less than 1kb" opinionated library. As far as Provider examples go: a simple Provider has no passed store: a Provider with a store then should keep that behavior It seems wrong to be able to "add functionality" to GlobalToggleStore with props on a Provider; so additional actions will only effect the Provider. At this point, the Provider is subscribing to a store and also providing its internal state. In the case where no store is passed, it still constructs a store just to subscribe to it. Provider could be even simpler and just provide its internal state with any action props wrapped in simple React This removes the clumsy binding and makes |
|
I think it should disregard other props when a store is given. I mean it could override of course, but that again would mutate the store. Anyway, generally i think it's best to make it as simple as possible, better removing a few options rather than having something that can do all but it's getting hard to guess at what it's actually doing. |
|
Your concern about overriding is why I made the change to provider. Any actions you pass to a provider will always only mutate the provider. In the same vein, any values you pass to a provider will always only be available to it's children. Passing a store only subscribes to it. If it's state has actions, they are just passed along - any store actions will always only mutate the store (which of course will notify the Provider and overwrite its initial values). I do think it was perfectly clear with: it uses the store or constructs a store with its props and can change it back. DetailsI added the extra state because of how my app previously did initial state for the repeated providers: ```js ```it could now do: <Provider store={FormSectionStore} drawersOpen={false} ... >So the one section has a different initial state, but all interaction with the store then sync it up with the rest of the app. I can already see that overridden actions get destroyed pretty quickly. It may be confusing as these are initial values and nothing indicates it - maybe this is just a prop to Provider (but this is additional api): <Provider store={SessionStore} initialState={{ drawersOpen: true }} ... > |
|
I reverted my change and also made sure not to change how subscribers would notified. I think this is mergeable if the coverage changes are fine. |
While looking for a way to fix #24, I wondered off, so I'm open to as big of suggestions as anyone wants to give.
Currently, external stores and Providers are a little confusing. The constructor for a Provider actually finishes constructing the "external" store. I understand this to be a two-way binding more now - the Provider's state is made external to React.
This could be improved upon, making the relationship explicit and subscribe-able. But that's not how I was originally using them or what I've done here...
This PR moves all the construction of external stores to
createStoreand introduces a new step in construction:wrapStateUpdateFunctions(state, store, callback).API Changes:
a store is ready for use after
createStore().<Provider />now only needs to subscribe to state changes and uses ordinary ReactsetState.Examples
Two Providers, one store (naive coder?):
https://codesandbox.io/s/jp46nlpk8y?view=preview