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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v2] useField: bound name to onChange and onBlur #1674

Open
neiker opened this issue Jul 12, 2019 · 11 comments 路 May be fixed by #2082
Open

[v2] useField: bound name to onChange and onBlur #1674

neiker opened this issue Jul 12, 2019 · 11 comments 路 May be fixed by #2082
Labels

Comments

@neiker
Copy link

neiker commented Jul 12, 2019

馃殌 Feature request

Current Behavior

I'm using useField on react-native and since TextInput has no name attribute, I need to bind name to handlers manually:

const TextField: React.SFC<TextFieldProps> = ({ name, ...props }) => {
  const [field] = useField(name);

  return (
    <TextInput
      {...props}
      value={field.value}
      onChangeText={field.onChange(name)}
      onBlur={field.onBlur(name)}
    />
  );
};

Desired Behavior

I spect to use onChange and onBlur handlers like this:

const TextField: React.SFC<TextFieldProps> = ({ name, ...props }) => {
  const [field] = useField(name);

  return (
    <TextInput
      {...props}
      value={field.value}
      onChangeText={field.onChange}
      onBlur={field.onBlur}
    />
  );
};

Suggested Solution

Bind name in formik.getFieldProps.
I think here:

formik/src/Formik.tsx

Lines 765 to 770 in 377c36e

const field: FieldInputProps<any> = {
name,
value: valueState,
onChange: handleChange,
onBlur: handleBlur,
};

It should be replaced with:

      const field: FieldInputProps<any> = {
        name,
        value: valueState,
        onChange: handleChange(name),
        onBlur: handleBlur(name),
};

Can this break useField for web users?

Possible workarounds

Create a useFieldNative:

import * as React from 'react';

import { useField, FieldMetaProps } from 'formik';

interface FieldInputNativeProps {
  value: string;
  onChangeText: (text: string) => void;
  onBlur: () => void;
}

export default function useFieldNative(
  name: string,
): [FieldInputNativeProps, FieldMetaProps<any>] {
  const [field, meta] = useField(name);
  const onChangeText = React.useCallback(field.onChange(name), [
    name,
    field.onChange,
  ]);
  const onBlur = React.useCallback(() => field.onBlur(name), [field, name]);

  const fieldNative: FieldInputNativeProps = {
    value: field.value,
    onChangeText,
    onBlur,
  };

  return [fieldNative, meta];
}


const TextField: React.SFC<TextFieldProps> = ({ name, ...props }) => {
  const [field] = useFieldNative(name);

  return <TextInput {...props} {...field} />;
};

Who does this impact? Who is this for?

react-native users

@sibelius
Copy link

sibelius commented Aug 6, 2019

this would be better for web as well

@stale stale bot added the stale label Oct 19, 2019
@kyle-johnson
Copy link
Contributor

At the least, need to update the react-native docs (I may find time for a PR later in the week).

This was a real head scratcher, since name is already being passed to useField.

@stale stale bot removed the stale label Nov 5, 2019
@kyle-johnson kyle-johnson linked a pull request Nov 30, 2019 that will close this issue
@stale stale bot added the stale label Jan 4, 2020
@zingerj
Copy link
Contributor

zingerj commented Jan 7, 2020

Seems like this wouldn't break any existing functionality, but would be super helpful for React Native users (like me!). At the very least a slightly modified useNativeField as proposed here would be much appreciated!

@stale stale bot removed the stale label Jan 7, 2020
@jaredpalmer
Copy link
Owner

I agree. Feel free to submit a PR.

@zingerj
Copy link
Contributor

zingerj commented Jan 7, 2020

Would you prefer extending existing functionality of useField to work for this, or adding a new dedicated hook, like useNativeField?

@jaredpalmer
Copy link
Owner

My guess is that this is actually breaking because people have messed with useField鈥檚 field.onChange although that was not my intention

@zingerj
Copy link
Contributor

zingerj commented Jan 7, 2020

Hmm okay, sounds like useNativeField would be safer then, but would that overlap too much with the upcoming useTextInput, assuming that all formik-native hooks would have this behavior built-in?

@jaredpalmer
Copy link
Owner

Add it to formik native

@zingerj
Copy link
Contributor

zingerj commented Jan 7, 2020

Happy to submit a PR, but just want to make sure I'm doing it right. Wouldn't this change be similar to (or the same as) cb71c04? Or are you recommending to do basically the same thing, but extract to formik-native?

@jaredpalmer
Copy link
Owner

Yes, but forgot I did that. Isn鈥檛 this fixed then?

@zingerj
Copy link
Contributor

zingerj commented Jan 7, 2020

Seemed like it was out of date so I implemented the same thing in #2176, will also update the docs and tests accordingly.

@stale stale bot added the stale label Mar 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants