chrisblossom / react-safe-universal-inputs

Fixes a React race condition when using controlled inputs combined with server-side rendering
https://react-safe-universal-inputs.herokuapp.com
10 stars 3 forks source link

Why is the value or checked prop required in the ref? #1

Open johnnyodonnell opened 6 years ago

johnnyodonnell commented 6 years ago

https://github.com/chrisblossom/react-safe-universal-inputs/blob/3e2b8b6c01f2dd3473a53c93887066bceea774c6/src/InputWrapper.js#L79

Currently my form inputs don't have value as a prop; and thus, this package does not work on my inputs. Is there a reason why value or checked is required?

chrisblossom commented 6 years ago

If you are not using a value / checked prop, you are using an uncontrolled component. This library is only meant to fix a race condition with SSR + controlled components. See https://reactjs.org/docs/forms.html#controlled-components.

If I am misunderstanding your question / issue / wrong about controlled components, please provide a minimal example.

johnnyodonnell commented 6 years ago

I'm using controlled components, but for a few specific inputs I don't have React setting the value.

For example, my password inputs have an onChange handler, but React doesn't set the value. This way if the user leaves the sign up page and comes back, the sign up form will have all fields filled in except for the password inputs.

This seems to be an uncommon use case, and possibly not a React recommended approach, but it does work pretty well for what I'm trying to accomplish.

chrisblossom commented 6 years ago

I am still not sure that is a controlled component. So your goal with this library is to call onChange in the event the input has changed prior to React being loaded?

I don't know how we'd see if a user has changed the value prior to React being loaded if the input state is not handled by React, but instead the native DOM. Maybe use a data attribute, such as data-initial-value.

I think with your use case might be better served with a standard componentDidMount and just clear the input every time the component is mounted.

Maybe I am still confused exactly how this input looks. Could you provide a minimal example?

chrisblossom commented 6 years ago

@johnnyodonnell Is this still an issue? If not, how did you solve the problem?

johnnyodonnell commented 6 years ago

This is still an issue. My workaround is to set the value attribute even though it's not the behavior I want.

I can give you an example to help better explain why I think the value attribute should be optional. The site I am building is stacks.courses

Steps:

  1. Go to stacks.courses
  2. Fill out the sign up form but don't submit the form
  3. Click the login button even though it is empty (this will take you to a dedicated login page)
  4. Return back to the sign up form by either (a) clicking the Stacks logo or (b) clicking the "sign up here" link

Desired result: The sign up form shows with first name, last name, and email still filled in. And password and confirm password are blank.

Actual result: The sign up form shows first name, last name, email, password, and password confirm all filled in.


If value is not a required attribute then I believe I can achieve the desired result.