Skip to content
This repository has been archived by the owner on Sep 7, 2020. It is now read-only.

Consider replacing redux in favor of react context #40

Closed
MoOx opened this issue Oct 26, 2015 · 18 comments · Fixed by #936
Closed

Consider replacing redux in favor of react context #40

MoOx opened this issue Oct 26, 2015 · 18 comments · Fixed by #936

Comments

@MoOx
Copy link
Owner

MoOx commented Oct 26, 2015

Since we don't do a lot of modifications (ie: no modification at all for now), we might consider replacing redux usage by react context (that are now documented http://facebook.github.io/react/docs/context.html).
That should be clearly enough to just "pass" some data (collection and page components references) that are declared during app initialization.

It don't really see any downside from the current usage.

Any thoughts? poke @bloodyowl

@bloodyowl
Copy link
Contributor

I'd really say it depends on what you try to achieve. statinamic already seems opinionated on some parts, and I see it more like a boilerplate than a library. if you want to provide a way to manage data across components whatever the tree structure is in this boilerplate, then I'd say keep redux. If you want it to be more of a library, keep it minimal, and just use your own provider and decorator.

@MoOx
Copy link
Owner Author

MoOx commented Oct 26, 2015

Statinamic is a collection of lib + a boilerplate generator + docs: you start with a boilerplate that use statinamic libs to help you generate a website with react and webpack. So Statinamic is opininated because of these 2 things (react and webpack). Except that nothing is mandatory (no required structures etc).
You can use the webpack config you want, your own loader, and any npm module you want in your components.

There is a minimal boilerplate that is required to allow some flexibility (choose your styles flavor (inline, css modules, sass...) and I am currently writing a helper to generate the minimal required boilerplate.

The idea is to allow to create a website very easily with one command $ statinamic setup: this will generate the boilerplate that use Statinamic libs. The only things that Statinamic uses are:

  • a registry to access page data
  • a registry to get available pages layouts (component references) that can be used from md header

Not a lot of things, no user interaction. That's what I was thinking about using react context instead of redux. Currently there is already a context provider to share some metadata about your project (eg: package.json to get homepage url, stuff like that) and I was thinking about using it for all the shared (read only - or sort of) data. The only interaction is: you click on a link, if page data are not in the store, we fetch a json to get those.

@thangngoc89
Copy link
Contributor

I use redux in almost every project so using redux is one the reasons why I use statinamic.

Also with redux, I can just add reducers that listen for default action of statinamic. Some example use cases: show a loading indicator when getting new page's info. show a error message when getting new page's info failed, ...

In the other hand, keeping redux would make statinamic too opinionated.

One more thing, if you decide to keep redux, you might need to use immutable.js (or similars) because redux store is holding all pages info which could lead to a performance issue on large page.

@MoOx
Copy link
Owner Author

MoOx commented Jan 24, 2016

show a loading indicator when getting new page's info. show a error message when getting new page's info failed, ...

I think thoses things should be possible without redux (and more easy to implement without using redux).

I will try to avoid redux and maybe will consider removing it.

@thangngoc89
Copy link
Contributor

I think we should consider this again. I don't see the benefit of using redux anymore. We aren't modify anything in the state (just add more page).

This should be a good alternative solution
https://github.com/heroku/react-refetch

@MoOx
Copy link
Owner Author

MoOx commented Feb 18, 2016

I think we should not need anything if we remove redux. All the logic for downloading is in PageContainer, maybe we can make a simple fetch/registry directly in here for now. And that might help for hot loading md.

@thangngoc89
Copy link
Contributor

It is your call. Removing redux would help us reduce the bundle size

@MoOx
Copy link
Owner Author

MoOx commented Feb 18, 2016

Totally. That's why I opened this issue in the first place :)

@thangngoc89
Copy link
Contributor

So. Shall we change status of this issue ?

@MoOx
Copy link
Owner Author

MoOx commented Feb 18, 2016

Done :)

@thangngoc89
Copy link
Contributor

I think that implement this would be easy, just mutate a window object. 💃

@MoOx
Copy link
Owner Author

MoOx commented Feb 29, 2016

@MoOx MoOx mentioned this issue Apr 27, 2016
@arlair
Copy link
Contributor

arlair commented May 6, 2016

I was investigating this tool to use with a MobX store app, so removing Redux is 👍 from me.

@MoOx
Copy link
Owner Author

MoOx commented May 6, 2016

Will be handled soon (hopefully)

@dab
Copy link

dab commented Jul 12, 2016

Any updates on this issue? It's really nice to have easy way to add your preferred state store. I'm also using mobx currently, and would like to add it to my static site app as well. It's not such simple to replace redux right now.
Thanks.

@MoOx
Copy link
Owner Author

MoOx commented Jul 12, 2016

I will try to work on that for the next release. Should not take that long.

@dab
Copy link

dab commented Jul 12, 2016

👍 thanks for the update

@bloodyowl
Copy link
Contributor

Going to be fixed with #925

@MoOx MoOx mentioned this issue Jan 12, 2017
Merged
@MoOx MoOx closed this as completed in #936 May 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants