callstack / react-native-paper

Material Design for React Native (Android & iOS)
https://reactnativepaper.com
MIT License
12.49k stars 2.05k forks source link

TextInput controlled/uncontrolled component warnings #2955

Open kidroca opened 2 years ago

kidroca commented 2 years ago

Current behaviour

Internally TextInput seems to always be passing a value to the native view Passing a defaultValue prop would raise the following warnings (only on react-native-web) though

Warning: TextInput contains an input of type text with both value and defaultValue props. Input elements must be either controlled or uncontrolled (specify either the value prop, or the defaultValue prop, but not both). Decide between using a controlled or uncontrolled input element and remove one of these props. More info: https://reactjs.org/link/controlled-components

Warning: A component is changing an uncontrolled input to be controlled. This is likely caused by the value changing from undefined to a defined value, which should not happen. Decide between using a controlled or uncontrolled input element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components

Expected behaviour

Support uncontrolled inputs

I want to use a TextInput without passing the value prop. I might pass a defaultValue: https://reactnative.dev/docs/textinput#defaultvalue

Useful for use-cases where you do not want to deal with listening to events and updating the value prop to keep the controlled state in sync.

Code sample

https://snack.expo.dev/@kidroca/paper-uncontrolled-text-input https://snack.expo.io/@kidroca/paper-uncontrolled-text-input

What have you tried

It seems it happens due to the ...rest spread here, when rest contains defaultValue it would be spread to the underlying element

https://github.com/callstack/react-native-paper/blob/58851e3376430b8c4278faa38f5b43339743784d/src/components/TextInput/TextInput.tsx#L437-L439

A workaround that I currently use is to use the render prop and render the react-native TextInput without passing the value

Could we maybe not pass the value down to the native field if we don't have to? The value is updated per each text change

https://github.com/callstack/react-native-paper/blob/58851e3376430b8c4278faa38f5b43339743784d/src/components/TextInput/TextInput.tsx#L376-L381

When we don't pass the value down to the native field? This way we carry less updates back and forth and have better performance. We can still keep it in local state to react to it's changes

Your Environment

software version
ios or android iOS 15.0.2
react-native 0.65.1
react-native-web ^0.17.1
react-native-paper ^4.9.2
node v14.18.1
npm or yarn npm v6.14.15
expo sdk n/a
github-actions[bot] commented 2 years ago

Couldn't find version numbers for the following packages in the issue:

Can you update the issue to include version numbers for those packages? The version numbers must match the format 1.2.3.

The versions mentioned in the issue for the following packages differ from the latest versions on npm:

Can you verify that the issue still exists after upgrading to the latest versions of these packages?

github-actions[bot] commented 2 years ago

Hey! Thanks for opening the issue. The issue doesn't seem to contain a link to a repro (a snack.expo.io link or link to a GitHub repo under your username).

Can you provide a minimal repro which demonstrates the issue? A repro will help us debug the issue faster. Please try to keep the repro as small as possible and make sure that we can run it without additional setup.

SleeplessByte commented 2 years ago

This is still a problem. Repro:

<TextInput
  defaultValue="My default value"
  onChangeText={() => {} }
  mode="outlined"
/>
SleeplessByte commented 2 years ago

Here is the patch-package patch as workaround. All it does is not apply value if defaultValue is present:

Object.prototype.hasOwnProperty.call(rest, 'defaultValue') ? undefined : this.state.value

Save as patches/react-native-paper+4.11.2.patch. Feel free to turn this into a PR and take credit.

diff --git a/node_modules/react-native-paper/lib/commonjs/components/TextInput/TextInput.js b/node_modules/react-native-paper/lib/commonjs/components/TextInput/TextInput.js
index 6a28b61..019acbf 100644
--- a/node_modules/react-native-paper/lib/commonjs/components/TextInput/TextInput.js
+++ b/node_modules/react-native-paper/lib/commonjs/components/TextInput/TextInput.js
@@ -346,7 +346,7 @@ class TextInput extends React.Component {
       ...rest
     } = this.props;
     return mode === 'outlined' ? /*#__PURE__*/React.createElement(_TextInputOutlined.default, _extends({}, rest, {
-      value: this.state.value,
+      value: Object.prototype.hasOwnProperty.call(rest, 'defaultValue') ? undefined : this.state.value,
       parentState: this.state,
       innerRef: ref => {
         this.root = ref;
@@ -359,7 +359,7 @@ class TextInput extends React.Component {
       onLeftAffixLayoutChange: this.onLeftAffixLayoutChange,
       onRightAffixLayoutChange: this.onRightAffixLayoutChange
     })) : /*#__PURE__*/React.createElement(_TextInputFlat.default, _extends({}, rest, {
-      value: this.state.value,
+      value: Object.prototype.hasOwnProperty.call(rest, 'defaultValue') ? undefined : this.state.value,
       parentState: this.state,
       innerRef: ref => {
         this.root = ref;
diff --git a/node_modules/react-native-paper/lib/module/components/TextInput/TextInput.js b/node_modules/react-native-paper/lib/module/components/TextInput/TextInput.js
index aee53ef..e21a31d 100644
--- a/node_modules/react-native-paper/lib/module/components/TextInput/TextInput.js
+++ b/node_modules/react-native-paper/lib/module/components/TextInput/TextInput.js
@@ -321,7 +321,7 @@ class TextInput extends React.Component {
       ...rest
     } = this.props;
     return mode === 'outlined' ? /*#__PURE__*/React.createElement(TextInputOutlined, _extends({}, rest, {
-      value: this.state.value,
+      value: Object.prototype.hasOwnProperty.call(rest, 'defaultValue') ? undefined : this.state.value,
       parentState: this.state,
       innerRef: ref => {
         this.root = ref;
@@ -334,7 +334,7 @@ class TextInput extends React.Component {
       onLeftAffixLayoutChange: this.onLeftAffixLayoutChange,
       onRightAffixLayoutChange: this.onRightAffixLayoutChange
     })) : /*#__PURE__*/React.createElement(TextInputFlat, _extends({}, rest, {
-      value: this.state.value,
+      value: Object.prototype.hasOwnProperty.call(rest, 'defaultValue') ? undefined : this.state.value,
       parentState: this.state,
       innerRef: ref => {
         this.root = ref;
diff --git a/node_modules/react-native-paper/src/components/TextInput/TextInput.tsx b/node_modules/react-native-paper/src/components/TextInput/TextInput.tsx
index 4641c34..26c8bcd 100644
--- a/node_modules/react-native-paper/src/components/TextInput/TextInput.tsx
+++ b/node_modules/react-native-paper/src/components/TextInput/TextInput.tsx
@@ -444,7 +444,7 @@ class TextInput extends React.Component<TextInputProps, State> {
     return mode === 'outlined' ? (
       <TextInputOutlined
         {...rest}
-        value={this.state.value}
+        value={Object.prototype.hasOwnProperty.call(rest, 'defaultValue') ? undefined : this.state.value}
         parentState={this.state}
         innerRef={(ref) => {
           this.root = ref;
@@ -460,7 +460,7 @@ class TextInput extends React.Component<TextInputProps, State> {
     ) : (
       <TextInputFlat
         {...rest}
-        value={this.state.value}
+        value={Object.prototype.hasOwnProperty.call(rest, 'defaultValue') ? undefined : this.state.value}
         parentState={this.state}
         innerRef={(ref) => {
           this.root = ref;
SleeplessByte commented 2 years ago

In 4.12.x new errors were introduced, which require a different patch.

// Use value from props instead of local state when input is controlled
const value = isControlled ? rest.value : uncontrolledValue;

// ...
<TextInputComponent
   // ...
   {...rest}
   value={value}
   // ...
/>

This incorrect assigns value when the component is uncontrolled.

Save as patches/react-native-paper+4.12.1.patch., delete the previous one.

diff --git a/node_modules/react-native-paper/lib/commonjs/components/TextInput/TextInput.js b/node_modules/react-native-paper/lib/commonjs/components/TextInput/TextInput.js
index 04a7c0f..887d2da 100644
--- a/node_modules/react-native-paper/lib/commonjs/components/TextInput/TextInput.js
+++ b/node_modules/react-native-paper/lib/commonjs/components/TextInput/TextInput.js
@@ -293,7 +293,7 @@ const TextInput = /*#__PURE__*/React.forwardRef((_ref, ref) => {
       editable: editable,
       render: render
     }, rest, {
-      value: value,
+      value: isControlled ? value : undefined,
       parentState: {
         labeled,
         error,
@@ -326,7 +326,7 @@ const TextInput = /*#__PURE__*/React.forwardRef((_ref, ref) => {
     editable: editable,
     render: render
   }, rest, {
-    value: value,
+    value: isControlled ? value : undefined,
     parentState: {
       labeled,
       error,
diff --git a/node_modules/react-native-paper/lib/module/components/TextInput/TextInput.js b/node_modules/react-native-paper/lib/module/components/TextInput/TextInput.js
index 4a4fb51..711129c 100644
--- a/node_modules/react-native-paper/lib/module/components/TextInput/TextInput.js
+++ b/node_modules/react-native-paper/lib/module/components/TextInput/TextInput.js
@@ -273,7 +273,7 @@ const TextInput = /*#__PURE__*/React.forwardRef((_ref, ref) => {
       editable: editable,
       render: render
     }, rest, {
-      value: value,
+      value: isControlled ? value : undefined,
       parentState: {
         labeled,
         error,
@@ -306,7 +306,7 @@ const TextInput = /*#__PURE__*/React.forwardRef((_ref, ref) => {
     editable: editable,
     render: render
   }, rest, {
-    value: value,
+    value: isControlled ? value : undefined,
     parentState: {
       labeled,
       error,
diff --git a/node_modules/react-native-paper/src/components/TextInput/TextInput.tsx b/node_modules/react-native-paper/src/components/TextInput/TextInput.tsx
index 2fc72d4..bcc4753 100644
--- a/node_modules/react-native-paper/src/components/TextInput/TextInput.tsx
+++ b/node_modules/react-native-paper/src/components/TextInput/TextInput.tsx
@@ -403,7 +403,7 @@ const TextInput = React.forwardRef<TextInputHandles, TextInputProps>(
           editable={editable}
           render={render}
           {...rest}
-          value={value}
+          value={isControlled ? value : undefined}
           parentState={{
             labeled,
             error,
@@ -438,7 +438,7 @@ const TextInput = React.forwardRef<TextInputHandles, TextInputProps>(
         editable={editable}
         render={render}
         {...rest}
-        value={value}
+        value={isControlled ? value : undefined}
         parentState={{
           labeled,
           error,
SleeplessByte commented 1 year ago

FYI, this is still present in 5.1.4.

Yarn 3 offers an out-of-box experience for patches, so if you're using that, here is how I generated and applied the patch for that version:

  1. Ensure Yarn 3: yarn set version stable
  2. Ensure .yarnrc.yml has nodeLinker: node-modules
  3. Ensure .gitignore doesn't ignore patches and releases
  4. yarn patch react-native-paper
  5. Follow the printed instructions and make changes
  6. `yarn patch-commit -s "/path/to/listed/folder"
  7. This generated the patch below with file path: .yarn/patches/react-native-paper-npm-5.1.4-df7f4a3886.patch
  8. This adds the following "resolutions" to package.json: "react-native-paper@^5.1.4": "patch:react-native-paper@npm%3A5.1.4#./.yarn/patches/react-native-paper-npm-5.1.4-df7f4a3886.patch"
  9. Run yarn install again to apply

(Apologies for the styling changes. It happened automatically because of the prettier config in the tree)

If you want to apply this patch yourself, you don't need to go through the above steps if you already use Yarn 3. Instead, copy the patch below to the same path, add the resolutions entry, and run yarn install.

diff --git a/lib/commonjs/components/TextInput/TextInput.js b/lib/commonjs/components/TextInput/TextInput.js
index 6aa8423edec6e1ccd09b4b5b929f1c72eb6fb296..bc535893243324f4cb48a4d739a7fc5eef78ddd9 100644
--- a/lib/commonjs/components/TextInput/TextInput.js
+++ b/lib/commonjs/components/TextInput/TextInput.js
@@ -274,7 +274,8 @@ const TextInput = (0, _forwardRef.forwardRef)((_ref, ref) => {
       render: render
     }, rest, {
       theme: theme,
-      value: value,
+      defaultValue: isControlled ? undefined : value,
+      value: isControlled ? value : undefined,
       parentState: {
         labeled,
         error,
@@ -308,7 +309,8 @@ const TextInput = (0, _forwardRef.forwardRef)((_ref, ref) => {
     render: render
   }, rest, {
     theme: theme,
-    value: value,
+    defaultValue: isControlled ? undefined : value,
+    value: isControlled ? value : undefined,
     parentState: {
       labeled,
       error,
diff --git a/lib/module/components/TextInput/TextInput.js b/lib/module/components/TextInput/TextInput.js
index 7c413a8ec10d5a6270d9b3da0cc15cf583624d55..ed5d3090b7c448028ded03949f150f6db3ed86a1 100644
--- a/lib/module/components/TextInput/TextInput.js
+++ b/lib/module/components/TextInput/TextInput.js
@@ -265,7 +265,8 @@ const TextInput = forwardRef((_ref, ref) => {
       render: render
     }, rest, {
       theme: theme,
-      value: value,
+      defaultValue: isControlled ? undefined : value,
+      value: isControlled ? value : undefined,
       parentState: {
         labeled,
         error,
@@ -299,7 +300,8 @@ const TextInput = forwardRef((_ref, ref) => {
     render: render
   }, rest, {
     theme: theme,
-    value: value,
+    defaultValue: isControlled ? undefined : value,
+    value: isControlled ? value : undefined,
     parentState: {
       labeled,
       error,
diff --git a/src/components/TextInput/TextInput.tsx b/src/components/TextInput/TextInput.tsx
index 1398d8b0046087577be80b636d6dc0161d0a967e..001c04b923364c7f518a1b4640cc84e6ffd499d0 100644
--- a/src/components/TextInput/TextInput.tsx
+++ b/src/components/TextInput/TextInput.tsx
@@ -1,4 +1,4 @@
-import * as React from 'react';
+import * as React from "react";
 import {
   Animated,
   LayoutChangeEvent,
@@ -6,20 +6,20 @@ import {
   TextInput as NativeTextInput,
   TextStyle,
   ViewStyle,
-} from 'react-native';
+} from "react-native";

-import { useInternalTheme } from '../../core/theming';
-import type { ThemeProp } from '../../types';
-import { forwardRef } from '../../utils/forwardRef';
+import { useInternalTheme } from "../../core/theming";
+import type { ThemeProp } from "../../types";
+import { forwardRef } from "../../utils/forwardRef";
 import TextInputAffix, {
   Props as TextInputAffixProps,
-} from './Adornment/TextInputAffix';
+} from "./Adornment/TextInputAffix";
 import TextInputIcon, {
   Props as TextInputIconProps,
-} from './Adornment/TextInputIcon';
-import TextInputFlat from './TextInputFlat';
-import TextInputOutlined from './TextInputOutlined';
-import type { RenderProps, TextInputLabelProp } from './types';
+} from "./Adornment/TextInputIcon";
+import TextInputFlat from "./TextInputFlat";
+import TextInputOutlined from "./TextInputOutlined";
+import type { RenderProps, TextInputLabelProp } from "./types";

 const BLUR_ANIMATION_DURATION = 180;
 const FOCUS_ANIMATION_DURATION = 150;
@@ -33,7 +33,7 @@ export type Props = React.ComponentPropsWithRef<typeof NativeTextInput> & {
    * In `outlined` mode, the background color of the label is derived from `colors?.background` in theme or the `backgroundColor` style.
    * This component render TextInputOutlined or TextInputFlat based on that props
    */
-  mode?: 'flat' | 'outlined';
+  mode?: "flat" | "outlined";
   left?: React.ReactNode;
   right?: React.ReactNode;
   /**
@@ -173,7 +173,7 @@ interface CompoundedComponent

 type TextInputHandles = Pick<
   NativeTextInput,
-  'focus' | 'clear' | 'blur' | 'isFocused' | 'setNativeProps'
+  "focus" | "clear" | "blur" | "isFocused" | "setNativeProps"
 >;

 /**
@@ -224,7 +224,7 @@ type TextInputHandles = Pick<
 const TextInput = forwardRef<TextInputHandles, Props>(
   (
     {
-      mode = 'flat',
+      mode = "flat",
       dense = false,
       disabled = false,
       error: errorProp = false,
@@ -249,7 +249,7 @@ const TextInput = forwardRef<TextInputHandles, Props>(
     );
     const [focused, setFocused] = React.useState<boolean>(false);
     const [placeholder, setPlaceholder] = React.useState<string | undefined>(
-      ' '
+      " "
     );
     const [uncontrolledValue, setUncontrolledValue] = React.useState<
       string | undefined
@@ -344,7 +344,7 @@ const TextInput = forwardRef<TextInputHandles, Props>(
         // Root cause:    Placeholder initial value, which has length 0.
         // More context:  The issue was also reproduced in react-native, using its own TextInput.
         // Workaround:    Set an empty space character in the default value.
-        setPlaceholder(' ');
+        setPlaceholder(" ");
       }

       return () => {
@@ -436,7 +436,7 @@ const TextInput = forwardRef<TextInputHandles, Props>(

     const { maxFontSizeMultiplier = 1.5 } = rest;

-    if (mode === 'outlined') {
+    if (mode === "outlined") {
       return (
         <TextInputOutlined
           dense={dense}
@@ -447,7 +447,8 @@ const TextInput = forwardRef<TextInputHandles, Props>(
           render={render}
           {...rest}
           theme={theme}
-          value={value}
+          defaultValue={isControlled ? undefined : value}
+          value={isControlled ? value : undefined}
           parentState={{
             labeled,
             error,
@@ -484,7 +485,8 @@ const TextInput = forwardRef<TextInputHandles, Props>(
         render={render}
         {...rest}
         theme={theme}
-        value={value}
+        defaultValue={isControlled ? undefined : value}
+        value={isControlled ? value : undefined}
         parentState={{
           labeled,
           error,
cjmling commented 8 months ago

Almost 2 year, and this issue still exist.

ggalmeida0 commented 2 days ago

I'm also running into this. Do we need more bandwidth to fix it? I can give it a stab if that's the case

SleeplessByte commented 2 days ago

You can take my patch and create a PR with it (git has tools for this too). Feel free to take credit.