-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Chore: Refactor to the new Context.unstable_read() API #814
Comments
Woah, didn't know about this API 😄That's awesome. Syntactically it reminds me of this babel plugin - https://github.com/Andarist/babel-plugin-jsx-adopt. Idea of creating "context getters" seems really cool (deletes unnecessary wrappers). import { fieldContext } from 'formik'
const MyField = props => {
const { value, error, touched } = fieldContext() // returning field bag
return ( /* jsx */)
} |
Unfortunately .read() and .set() only works in render phase functions right now. Womp. Womp. |
Hola! So here's the deal, between open source and my day job and life and what not, I have a lot to manage, so I use a GitHub bot to automate a few things here and there. This particular GitHub bot is going to mark this as stale because it has not had recent activity for a while. It will be closed if no further activity occurs in a few days. Do not take this personally--seriously--this is a completely automated action. If this is a mistake, just make a comment, DM me, send a carrier pidgeon, or a smoke signal. |
Closing. This will not be final API. Stay tuned for the real thing near React Conf in a few weeks. |
Current Behavior
Currently, we use
connect()
to wire up components to Formik's context. This creates an extra node for<Field>
,<Form>
, and<FastField>
becauseconnect()
is simply a wrapper around the<FormikContext.Consumer>
render prop.Desired Behavior
Remove
connect()
usage internally and instead use Andrew's forthcomingContext.unstable_read()
(facebook/react#13139) which allows reading context within any render-phase function.Suggested Solution
Pretty straight forward. We rename
<XXXXImpl>
to just<XXXX>
and get rid ofconnect()
usage internally (i.e. remove my poorly typed HoC TypeScript nonsense at the bottom of the files). We will still needinterface FormikContext
because I expect that the TS type signature for.unstable_read()
will take a TS generic.Who does this impact? Who is this for?
We will still export
connect()
so no breaking changes there. However, enzyme tests may break AGAIN because of these changes because we are going to be removing a node from the tree for each component (no more FieldImpl, just Field). 🤣 🤣. We can now go back to shallow rendering instead of mount though!Additional context / Discussion
<Field>
to an SFC and moving all the class functions down into render. I need to ask Dan/Andrew about pros / cons.<Field>
to create their own custom components? I'm not sure where these would get defined? Maybe ingetDerivedStateFromProps
in Formik? idk. Need to ask.The text was updated successfully, but these errors were encountered: