Pass options to devtools

[anilanar]

Although it is convenient to include devtools integration in this library, there's a fair question to ask here: is it this library's responsibility to do it? If it's done for technical reasons, I think we need a way to pass options to the devtools' compose function.

Particularly action blacklisting, state sanitization, max-age are invaluable for large apps.

In addition, different apps have different needs (e.g. using devtools in production etc.). There are infinitely many possible ways to configure devtools. What if createStore took a compose function as a parameter?

Another alternative (as seen in other issues/questions submitted) is to have an additional API that exports enhancers/middlewares for dynamic module management and let users seeking more freedom use createStore from redux.

If you would like to go with the first option (adding compose param to createStore), I can make a PR. I suggest that createStore’s first argument is made an options object in that case e.g. createStore<...>(options: { compose, initialState, enchancersEtc }, ...modules). I’m typing from mobile so this is not a type signature 😄

It seems it’d be better if this were done after #55 is merged. Nevertheless this will be a breaking change.

[anilanar]

For now, I have a fork that makes createStore take compose function as an argument.

[navneet-g]

@anilanar Please feel free to send a PR.

[navneet-g]

@anilanar what if we create an option to createStore that tells it to not compose with devtools, and you can pass devtools as an enhancer in createStore?

[anilanar]

@navneet-g Devtools' compose function iterates all enhancers and generates an instanceId for each intermediate store created by enhancers and stores them somewhere:

https://github.com/zalmoxisus/redux-devtools-extension/blob/master/src/browser/extension/inject/pageScript.js#L383-L400

I don't understand why it needs to do that though. So createStore({}, [reduxDevtoolsCompose(/* enhancers */)]) would make devtools miss enhancers added by redux-dynamic-modules' own createStore.

[dbartholomae]

Any way I can help here? This is a major hindrance for us using this package now :-/

[anilanar]

I don’t see a way to make this a non-breaking change.

If maintainers are fine with a breaking change, and if they propose the new API, I can make a PR. Otherwise I don’t want to waste time on bikeshedding and on a PR.

[navneet-g]

@abettadapur what if we take a compose function as input? @dbartholomae @anilanar will that help?

[dbartholomae]

Taking a function sounds great! And if enhancers is a function instead of a middleware, dev-tools are not included by default. If this approach is fine, I could try to write a PR.

[anilanar]

I propose:

// The following plus 8 overloads for `DeepPartial<union of module states>`

interface StoreOptions<S> {
  initialState: S,
  enhancers?: StoreEnhancer[],
  extensions?: IExtension[],
  compose?: typeof reduxCompose,
  modules: IModule<any>[],
}

export declare function createStore<S>(options: StoreOptions<S>): IModuleStore<S>;

It might be possible to avoid overloads with some Typescript 3.0+ type-level wizardry: microsoft/TypeScript#26058 (comment)

If someone's into it, it may be possible to take a tuple of modules like <M extends IModule<any>[]> and convert it to tuple of states with { [K in keyof M]: M[K] extends IModule<infer S> ? S : never } and then convert the tuple of states to intersection of states using the method provided in the link. Finally using initialState: DeepPartial<CalculatedState> and overloads are gone.

[anilanar]

I'm working on this now.

[dbartholomae]

Any news on the PR? Looking forward to this!

[anilanar]

Sorry I’ve been on a long vacation with no access to computers, will handle this when I’m back. On 23. Aug 2019, 04:40 +0300, Daniel Bartholomae <[email protected]>, wrote: Any news on the PR? Looking forward to this! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#65?email_source=notifications&email_token=AAFYASGQKHWPRAPWCKS4RSTQF45ZLA5CNFSM4G36CQF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD463WDI#issuecomment-524139277>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAFYASGZIN7TZ74TNESRPYDQF45ZLANCNFSM4G36CQFQ>.