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

Expose formikbag as imperative methods #1972

Merged
merged 3 commits into from
Jan 13, 2020
Merged

Expose formikbag as imperative methods #1972

merged 3 commits into from
Jan 13, 2020

Conversation

brunohkbx
Copy link
Contributor

Motivation

Since Formik 2 is built on top of React Hooks it was not possible to attach a ref and consequently, some people can't upgrade to v2. It resolves #1603.

My first approach was to use React.forwardRef like that:

export const Formik = React.forwardRef(<
  Values extends FormikValues = FormikValues,
  ExtraProps = {}
>(props: FormikConfig<Values> & ExtraProps, ref?: any) => {

But it breaks the interface FormikConfig giving the following errors:

Types of property 'render' are incompatible.
Types of property 'onReset' are incompatible.
Types of property 'valiadte' are incompatible.

'FormikValues' is assignable to the constraint of type 'V', but 'V' could be instantiated with a different subtype of constraint '{}'.

I've managed to make it work when I changed FormikProps<Values> to FormikProps<any> on render?, onReset, validate, but I don't know how to properly fix these types 😞

Do you guys have any thoughts on this? I would appreciate any feedback 😬

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit dee856e:

Sandbox Source
Formik v2 Template Configuration

@jaredpalmer
Copy link
Owner

I think this should work?

@jaredpalmer
Copy link
Owner

@johnrom thoughts?

@johnrom
Copy link
Collaborator

johnrom commented Oct 31, 2019

I'm very much in dotnet-land right now, but if you want feedback on the TypeScript I think you know my answer :). There should be a proper type (typeof formikBag ?) instead of any.

export type FormikRef<Values> = FormikBag<Values>; // change as necessary
export interface FormikConfig<Values> extends FormikSharedConfig {
  innerRef: React.MutableRefObject<FormikRef<Values>>;
}

Additionally, I don't know that a reference to the FormikBag is equivalent to a "reference" to the Formik component, and I think there are better ways to access the Formik API than prop drilling -- there is discussion around this on the issue linked above. Here's an example using Context. It's verbose, but with the benefit of being completely customizable.

type FormikSubmitContext = {
    registerOnSubmit: (handler: FormikHandlers['onSubmit']) => void;
    onSubmit?: FormikHandlers['onSubmit'];
}
const formikSubmitContext = React.createContext<FormikSubmitContext | undefined>();
const MyParentComponent = () => {
    const [onSubmit, registerOnSubmit] = React.useState<FormikHandlers['onSubmit']>();
    return <formikSubmitContext.Provider value={{
        onSubmit,
        registerOnSubmit
    }}>
        {/* imagine these using actual fn components */}
        <Formik>
            {formikProps => {
                const myContext = React.useContext(formikSubmitContext);
                
                if (!myContext) throw new Error("no context");

                React.useEffect(() => {
                    myContext.registerOnSubmit(formikProps.onSubmit);

                    return () => myContext.registerOnSubmit(undefined);
                }, [formikProps.onSubmit, myContext.registerOnSubmit])
            }}
        </Formik>
        {() => {
            const myContext = React.useContext(formikSubmitContext);

            return <button onClick={myContext.onSubmit}>Submit</button>
        }}
    </formikSubmitContext.Provider>
}

@brunohkbx
Copy link
Contributor Author

@johnrom thanks for the review.
@jaredpalmer Which approach do you prefer? Should we use a context instead?

@belgamo
Copy link

belgamo commented Nov 20, 2019

Any updates on this?

@jaredpalmer
Copy link
Owner

Why can't we just do useImperativeHandle?

@brunohkbx
Copy link
Contributor Author

I'm ok with this

@ksweetie
Copy link

@jaredpalmer Is the path forward here to do useImperativeHandle and properly type it? I can take a stab at it if that's what we want to do.

@jaredpalmer
Copy link
Owner

@ksweetie yes

@jaredpalmer
Copy link
Owner

And update the PR to deal with Lerna

@jaredpalmer
Copy link
Owner

@ksweetie want to take a stab at this?

@drivasperez
Copy link
Contributor

Is there any progress on this? It would make my life easier and I don't mind taking it on, but I don't want to swoop in if it's already being worked on.

@brunohkbx
Copy link
Contributor Author

brunohkbx commented Jan 9, 2020

I'll fix the conflicting files

@vercel vercel bot temporarily deployed to Preview January 13, 2020 19:25 Inactive
@jaredpalmer jaredpalmer changed the title feat: expose formikbag as imperative methods Expose formikbag as imperative methods Jan 13, 2020
@jaredpalmer jaredpalmer merged commit f80c36c into jaredpalmer:master Jan 13, 2020
@vercel
Copy link

vercel bot commented Jan 13, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/jared/formik-docs/h6sn0i51f
✅ Preview: https://formik-docs-git-fork-brunohkbx-feat-expose-formikbag.jared.now.sh

@brunohkbx brunohkbx deleted the feat/expose-formikbag branch January 13, 2020 19:31
@rally25rs
Copy link

rally25rs commented Jan 20, 2020

Using

formik 2.1.2
react 16.12.0

if I do:

const formikRef = React.useRef();

<Formik ref={formikRef}>

i still get:

  console.error node_modules/react-dom/cjs/react-dom.development.js:530
    Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?

    Check the render method of `ForwardRef(FormImpl)`.
        in Formik (created by ForwardRef(FormImpl))

and formikRef.current is undefined

Is this not the right way to be using this now? It used to work in a past version of react and formik, just going through a round of upgrading all my deps...


Edit:

Nevermind, now you use innerRef instead of ref.

const formikRef = React.useRef();

<Formik innerRef={formikRef}>

It also doesn't expose the same object that the old versions used to as ref (you used to have to call .getFormikActions() to get to some of the functions), but that's fine. Crisis averted 😆

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.

[v2] <Formik /> ref prop
7 participants