-
Notifications
You must be signed in to change notification settings - Fork 376
feat(simple redux): implement simple redux helpers to register and remove reducers #358
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
Conversation
|
I don't know about the lock files to package.json should they be part of this PR or not? |
…move reducers affects: @patternfly/simple-redux
|
this is an interesting topic - and I think it would be great to have some responses from all the React product teams interested in consuming. I would like to consider it some more, but in short, I think we should consider how this will impact future products (and it's great that we can expose this as an individual package now to isolate it). Here is a few relevant links... The future of Redux (likely to be more intertwined w/ React core, from a Redux maintainer): How Twitter handled code splitting Redux modules in a similar way: |
|
@priley86 didn't knew about the future of redux, for me Redux is quite alive 😃 and yeah me and @vojtechszocs wrote this and while I was checking how others did it same page with how twitter did it. And totaly I would like other teams to talk about this and check this out! I just wanted for others to know that something like this exists so they can check it ou. |
|
@karelhala interesting idea, looks like a lot of effort went into this, much appreciated! However, not entirely convinced this is needed since it does tie PF-React to the overhead of keeping up with a specific state management preference. This in turn feels like a opinionated topic best reserved for an app. Given the potential change in React core we may be better served with some form of example/demo/readme/guide only indicating this potential path Curious about other opinions out there... |
|
@cdcabrera ^^ thanks...i tend to agree. so considering this a bit further on my side... I personally prefer this repo move towards the upcoming React Suspense API (which will soon be released in React core). You can read more about this here: There is also UX implications around showing loading states so I highly recommend watching this video if you want to learn about it. The React team recently merged the PR for Suspense to master branch, and it looks like it is slated for upcoming release. It seems that @vojtechszocs 's presentation yesterday was also more similar to this API (using render props to specify child context while loading data). I could still see Redux as a useful tool in handling global application state, but that need should become less and less (thus I'm kind of opposed to introducing more Redux extensions in PF React). It seems that code splitting/component splitting the data layer properly would achieve the same thing...and I prefer to target core framework APIs for this. additional thoughts? |
|
Well the idea behind this to be part of PF-react repository is to be exposed as much as possibly. I understand that we don't want to bind ourselves only with redux, this is just optional for some teams if they want to use redux show them that they can hot swap reducers on the fly. Similiar thing could be written by someone else who uses flux or any other state management tool. Please do not take this as core thing, but rather option for others to use, should they decide they want to use hot swap of reducers. I have no intent to make this part of PF-react core, however if you don't want to have this as part of PF-react codebase I understand it. I'd port this over to some other repository, but we should have some kind of hub or place where to show to other teams any helper functions and other things and let them decide which to use. I feel like having this as part of PF might be good since PF is starting to grow out of only visual components and it could provide more packages with some neat functions which would help others. And yes it makes sense to move towards React Suspense API, but only for components, this does not have anything to do with any components. However huge applications would still need to hold their state somewhere, I don't think that RS API will replace any state logic libraries (Redux, Flux, Reflux, ...whateverux). |
|
@karelhala Would you still like this PR to be considered? |
|
@jeff-phillips-18 I still think that it could be interesting for others to see how redux can be used and set up in simple as possible way (mapping reducers hash is super usefull). Considering this implementation I think it would probably be better to follow with But if you and others think that PF should stay away from redux implementation I guess I could create custom repository with these helpers and we could probably just link it either from storybook or from somwhere else. |
|
I'd be ok with it being in its own package in this repo. |
|
Moving to closed for now until @karelhala can move the code into a separate package within the monorepo. |
Affects
What:
This PR introduces new package called
@patternfly/simple-reduxit contains multiple helper functions to make easier work with redux + it allows adding and removing reducers on the fly.This is brought from http://manageiq.org/ project where this thing was introduced by @karelhala and @vojtechszocs.
I am proposing this to be part of patternfly-react repo because it will get much bigger attention in this repository. And it might be helpfull to other teams struggling with redux.
This package contains thorough documentation in README file.
Alongside this PR I have introduced a way of adding title and description which is not realying on PF or bootstrap.
Link to Storybook:
simple-redux storybook
Additional issues: