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

State Reducer / Middleware #637

Closed
wants to merge 5 commits into from
Closed

State Reducer / Middleware #637

wants to merge 5 commits into from

Conversation

jaredpalmer
Copy link
Owner

This is proof of concept state reducer. I didn't quite finish exposing all state changes (still have to do submit and reset). Anyways, I thought it was enough to get some feedback. I tried to keep things elm like. This whole thing would be soooo much easier with pattern matching, but i digress. Here is my state reducer attempt.

Related: #401

<Formik reducer={(prevState, nextState, actions) => nextState)} ... />

payload: FormikErrors<Values>;
};

export function reducer<Values>(
Copy link
Owner Author

@jaredpalmer jaredpalmer May 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...so this file lays the groundwork for one day porting Formik to other JS frameworks. Hmmm. bitmoji

@slightlytyler
Copy link
Collaborator

slightlytyler commented May 21, 2018

Seems simple but not sure I see the utility in it. Looks like it has to run synchronously so not really middleware. The logger example is an ok use case but the persist one would get a 👎from me in code review if I saw it. The reducer wouldn't be my choice for effect handling, seems more idiomatic to handle that in a lifecycle.

What are some more "real world" use cases for this? Making the library extensible is 👌but I'd worry that this feature might not be prescribing good patterns

@slightlytyler
Copy link
Collaborator

If the goal is effect handling you should go full middleware imo

@slightlytyler
Copy link
Collaborator

Also the more I think about this the more it feels like a state-centric solution in a library that I believe to be component-centric

@jaredpalmer
Copy link
Owner Author

With respect to reducer prop, I see the a blessed way to open Formik's state / effects up to user land without expanding the core API. Example use cases:

  • Add warning
  • Autosave without another component

tbh I hadn't thought about state vs. component approach until you just articulated it so concretely. I agree 100% that formik is and should continue to be component-centric. Making me dislike this code because it mixes the 2 rather poorly.

However, the reducer prop and refactoring to a internal reducer don't need to go together. What are your thoughts on using the state reducer internally? This could give us logging and dev tools, but not sacrifice component-centricity.

@slightlytyler
Copy link
Collaborator

slightlytyler commented May 21, 2018

If you think it's a better internal API then by all means. I am curious to see a better effect handling story which seems to be at the core of what this is getting at. I see in the discussion that a few ideas have been explored.

Things I'd consider:

  1. Just using another component and the lifecycles. The component lifecycles are really powerful. Libraries like react-router showed us that sometimes you don't need your own effect handling story. We already have a great component API to build with. Maybe we just need better guides on what these composition patterns look like?
  2. Middleware. If you want to move towards a more declarative external API a true middleware pattern, like in redux, is the correct place to implement effect handling. This is a really big solution and might lead to asking yourself the question "Am I just reimplementing redux?". Personally, I think the imperative external API makes a lot of sense so not really a fan of this one.
  3. Handlers. I see there was some discussion on handlers like onChange. I'd argue this is also a proper place to implement effect handling and follows the controlled input API that most React users already know.

Are there other use cases or effect handling stories that I've missed?

@jaredpalmer
Copy link
Owner Author

Agree with all three points.

@jaredpalmer
Copy link
Owner Author

Going to close for now.

@jaredpalmer jaredpalmer deleted the feature/reducer branch December 12, 2019 19:25
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.

2 participants