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

allow StoreEnhancerStoreCreator to have 3rd param #2175

Closed
wants to merge 1 commit into from

Conversation

TotooriaHyperion
Copy link

createStore with middleware should allow a third param to pass enhancers in.

createStore with middleware should allow a third param to pass enhancers in.
@aikoven
Copy link
Collaborator

aikoven commented Jan 3, 2017

See #2130

@timdorr
Copy link
Member

timdorr commented Jan 5, 2017

Can someone with some TS experience confirm this looks good?

@aikoven
Copy link
Collaborator

aikoven commented Jan 6, 2017

The code looks good, but is it correct? Is there a need for enhanced store creator to accept another enhancer as third argument? Consider this:

const myEnhancer = createStore => (reducer, preloadedState, enhancer) => {
  let store = createStore(reducer, preloadedState, enhancer);
  // do something with store
  return store;
};

When using myEnhancer like this:

const store = createStore(reducer, preloadedState, myEnhancer);

this results in a call myEnhancer(createStore)(reducer, preloadedState), so enhanced store creator doesn't get enhancer argument.

The only way I see when it could be useful is to somehow modify enhancer argument, which would be possible if we used it like this:

const enhancedCreateStore = myEnhancer(createStore);
const store = enhancedCreateStore(reducer, preloadedState, someOtherEnhancer);

But I don't think that implementation of store enhancer should be dependent on the way it's used. Am I wrong?

@timdorr
Copy link
Member

timdorr commented Oct 6, 2017

Closing

@timdorr timdorr closed this Oct 6, 2017
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.

3 participants