[api] add proxy modules for storage, state reconcilers, and react integration

[yordis]

c. All constants (ex: {REHYDRATE, PURGE}) has moved to redux-persist/lib/constants instead of redux-persist/constants

I don't get why we have to put that ugly lib in the path, is like going backward on the package in that term.

I really don't see the point. Now we have that lib everywhere, which makes the package to look really ugly IMO and non ES6 module based, more like something that some tool is forcing you to do.

I hope is not late for fix that because it is a heart breaking to see this happening 😥

Could you remove that? 🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏

[rt2zz]

I absolutely would like to remove that and not just for constants but all deep imports. The only question is what is the best alternative pattern? I know @Andarist has been thinking about this issue for some time.

Open to suggestions, hopefully one that does not involve maintaining a separate es5 constant file outside of src.

[yordis]

@rt2zz it was working before, no?! Just do whatever you were doing 😄

[Andarist]

The only (known to me) possibility to have those 'cherry-picked' modules is to create proxy directories with package.json - like redux-persist had in the past.

I have wanted (and still want to) create a package to automate creation of those packages before publication or build steps. Just hadn't found enough time for this yet. I will try to sit down next weekend and create MVP for this.

You can also ofc reexport your constants in the main entry point (src/index.js) so one could just do import { constants } from 'redux-persist'.

You could also consider using rollup to build ur package as flat bundle.

[yordis]

@Andarist what would be the downside of rollup, it seems that a lot of people use it for this.

P.S: has been a long time without being involve in the JS so, what would happen if I use rollup on using your rollup version. Will rollup bundle just the piece of the app that I used? I forgot the name of that 😭

[rt2zz]

cool, sounds like proxy modules is probably the way to go for now then. We might export all constants out of the main module (i.e. import { REHYDRATE } from 'redux-persist) and then provide proxy modules for integration and storage.

[yordis]

@rt2zz is this still valid?

[rt2zz]

constants are now available from the root module. Still open because we need to add proxy modules for storage and state reconcilers

[Andarist]

@rt2zz could u elaborate on what proxy modules you'd like to have exactly? I might provide a PR for this.

[rt2zz]

@Andarist open to suggestion, but I will list the files that I think are considered to be part of the public api:

• ./storage/index.js (although perhaps we should rename this to local.js
• ./storage/session.js
• ./stateReconciler/hardSet.js
• ./stateReconciler/autoMergeLevel1.js
• ./stateReconciler/autoMergeLevel2.js
• ./integration/react.js (or should we expose this top level as just 'redux-persist/react'?)

[Andarist]

But are there any particular reasons why they need a proxy module and why having them just reexported from the main entry point is problematic?

Maybe the react one as its optional in a sense (maybe the same thing applies to storage too) deserves a separate entry, but Im not sure about the others (though im not familiar with redux-persist@5). When it comes to separate entries there are also 2 options:

• proxy modules
• scoped packages

[rt2zz]

@Andarist with scoped packages, we would need to publish separate npm modules for each correct?

As I understand it, there are two reasons we use proxy modules:

1. in the case of integration/react to avoid issues with the "undeclared" dependency of react. Of course even having the undeclared dependency at all is bad smell, but I hate to publish a separate module for what is an extremely common use case.
2. in the case of stateReconciler and storage, it is in order to avoid possible module bloat in the case that the consumer does not have tree shaking.

I could be easily convinced that 2 is not a valid reason - I just dont know the numbers. How many users have tree shaking bundlers? under what circumstances might tree shaking fail? etc.

What do you recommend @Andarist ?

[Andarist]

@Andarist with scoped packages, we would need to publish separate npm modules for each correct?

Correct.

in the case of integration/react to avoid issues with the "undeclared" dependency of react. Of course even having the undeclared dependency at all is bad smell, but I hate to publish a separate module for what is an extremely common use case.

Yeah, this is unfortunately a downside of using proxy modules - deps/peerDeps are a "headache" but as you have mentioned this is really not that much of an issue as this is a common use case AND you make it quite explicit when you force people to use redux-persist/integration/react (although personally I would prefer to have it shortened to redux-persist/react 😉 )

in the case of stateReconciler and storage, it is in order to avoid possible module bloat in the case that the consumer does not have tree shaking.

There are cases when tree-shakers (mainly webpack, rollup has way better algo) can get penalized by this approach. Personally I wouldn't really care that much about users without tree-shaking. At this moment webpack@4 is already in beta and tree-shaking is here since webpack@2. Can't worry about all releases all life, better to just support newer versions and push people into migrating - that benefits whole ecosystem. That's my personal view though 😅

under what circumstances might tree shaking fail? etc.

This is really hard to demonstrate, especially as it might differ between tools and their versions. I've stumbled once on what I thought was trivially tree-shaken, but it wasn't. You can checkout the issue and simple sample repo here.

You can also read this thread if you are interested in seeing what prevents tree-shaking (it's not meant as complete list or anything, but it demonstrates some additional issues).

My rule of thumb nowadays is to distribute flat bundles with rollup and let tree-shakers do their job. Flat bundles generally result also in less bytes being included in the application bundles, because things like babel helpers are shared within flat bundle boundary, it's easier to tree shake them, it's easier to minify them and possibly gzip too. Situation will get better (to some extent) when webpack@4 gets released because it will try to scope-hoist (flat bundle is scope hoisted bundle, so its generally the very same thing) in production by default. I suspect though that it might still be better to distribute flat bundles all along, even if only as some kind of pre-done scope hoisting - if we do when distributing the library, then bundlers won't have to do that when compiling (so it may be seen as some form of "caching").

Things are getting more complicated if we want to distribute flat-bundles AND proxy modules. Complexity may vary if proxy modules are completely separate thing from the main entry or are they sharing some code between each other. This can be configured though, especially with hot new experimentalCodeSplitting option of rollup. Can try to achieve this with redux-persist if you want.

What do you recommend @Andarist ?

Pretty much what I've described before - flat bundling all the things + trying to leverage experimentalCodeSplitting to create proxy modules

[rt2zz]

wow I had no idea how deep this went! great reads.

So I am 100% onboard with going flat bundle conceptually.

I am concerned that this line in storage will cause issues (dynamic property access of window). Maybe integration/react and storage are simply two exceptions that do not get flat bundled? This also mirrors the fact that most users do not need to import from storage as AsyncStorage for native and localForage for web are recommended.

[Andarist]

I'll try to cook something up and we'll be able to discuss further under the PR

[yordis]

@rt2zz @Andarist check the PR, hopefully you will no need to do any proxy package anymore.