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

[React 15.2.0] Additional props warning #1249

Closed
erikras opened this issue Jun 30, 2016 · 85 comments
Closed

[React 15.2.0] Additional props warning #1249

erikras opened this issue Jun 30, 2016 · 85 comments

Comments

@erikras
Copy link
Member

erikras commented Jun 30, 2016

It will be interesting to see, when React 15.2.0 is released, how many of these warnings redux-form generates. More info and discussion here from the page the warning links to:


The unknown-prop warning will fire if you attempt to render a DOM element with a prop that is not recognized by React as a legal DOM attribute/property. You should ensure that your DOM elements do not have spurious props floating around.

There are a couple of likely reasons this warning could be appearing:

  1. Your component is transferring its own props directly to a child element (eg. https://facebook.github.io/react/docs/transferring-props.html). When transferring props to a child component, you should ensure that you are not accidentally forwarding props that were intended to be interpreted by the parent component.
  2. You are using a non-standard DOM attribute, perhaps to represent custom data. Custom data attributes should always have a data- prefix, as described in https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Using_data_attributes
  3. React does not yet recognize the attribute you specified. This will likely be fixed in a future version of React. However, React currently strips all unknown attributes, so specifying them in your React app will not cause them to be rendered.

Props like valid, invalid, dirty, and pristine might cause warnings for cases where someone is just destructuring the field object into an input, i.e. <input type="text" {...firstName}/>.

@ooflorent
Copy link
Contributor

I'm affraid this change will lead to a massive amount of issues created here... Unfortunately we can't do much to prevent this. One solution in v6 is to provide a new input component.

const InputProps = [
  // HTML attributes
  "placeholder",
  "type",
  "value",
  // Event listeners
  "onBlur",
  "onChange",
  "onFocus",
]

const InputWrapper = props => 
  React.createElement("input", _.pick(props, InputProps))

export default InputWrapper

And to make it the default component of Field:

Field.defaultProps = {
  component: InputWrapper,
}

@ooflorent
Copy link
Contributor

Another workaround is to instrument component if it is passed as a String (input or select).

@erikras
Copy link
Member Author

erikras commented Jul 1, 2016

Results of first test:


Warning: Unknown props active, dirty, error, invalid, onUpdate, pristine, touched, valid, visited, asyncValidating on <input> tag. Remove these props from the element. For details, see https://fb.me/react-unknown-prop


It's amusing how accustomed whoever wrote that warning message is with GitHub Markdown that they used backquotes.

@erikras
Copy link
Member Author

erikras commented Jul 1, 2016

What if you could do something like this:

import { reduxForm, Field, filterProps } from 'redux-form'

const renderTextField = field => 
  <div>
    <input type="text" {...filterProps(field)}/>
//                         ^^^^^^^^^^^
    {field.touched && field.error && <div>{field.error}</div>}
  </div>

const MyForm = props =>
  <form onSubmit={props.handleSubmit}>
    <Field name="whatever" component={renderTextField}/>
  </form>
export default reduxForm({ form: 'myForm' })(MyForm)

Seems like the least a library designed to destructure objects into dom components could do...

@gaearon
Copy link
Contributor

gaearon commented Jul 1, 2016

Maybe pickDOMProps to make it clearer what it does.

@tylergoodman
Copy link

Is nesting the input-specific props an option? eg.

{
  input: {
    active
    autofill
    checked
    onBlur
    onChange
    onDragStart
    onDrop
    onFocus
    onUpdate
    value
    visited
  }
  autofilled
  dirty
  error
  initialValue
  invalid
  name
  pristine
  touched
  valid
}

@erikras
Copy link
Member Author

erikras commented Jul 2, 2016

@tylergoodman That is an option, yes.

@gaearon
Copy link
Contributor

gaearon commented Jul 2, 2016

@tylergoodman’s suggestion looks cleanest.

@erikras
Copy link
Member Author

erikras commented Jul 2, 2016

@gaearon Stinks of "breaking change", though.....

@gaearon
Copy link
Contributor

gaearon commented Jul 2, 2016

You could..

  1. Add input to both v5 and v6 without removing old props
  2. Remove old props in v6 final

@erikras
Copy link
Member Author

erikras commented Jul 4, 2016

Ooh, for a moment there, I thought I might be clever and make the redux-form metadata props (touched, pristine, error, etc.) not enumerable on the props object. That way they would not be copied when destructuring into the input component. Turns out React.createElement is enumerating through them, so they don't get through. Oh well...

@gaearon
Copy link
Contributor

gaearon commented Jul 4, 2016

Nah, smart tricks are only going to make things worse. Please fix the underlying issue. 😉

@erikras
Copy link
Member Author

erikras commented Jul 4, 2016

I wasn't too fond of it it originally, but after implementing it, @tylergoodman's suggestion is growing on me.

const renderField = field => (
  <div>
    <label>{field.input.placeholder}</label>
    <div>
      <input {...field.input}/>
      {field.touched && field.error && <span>{field.error}</span>}
    </div>
  </div>
)

The important thing is that the passthrough props go into the input key.

<Field name="username" component={renderField} type="text" placeholder="Username"/>
//                           Those guys --->   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@gaearon
Copy link
Contributor

gaearon commented Jul 4, 2016

Yea, IMO it’s the most elegant and clean solution. 👍

@erikras
Copy link
Member Author

erikras commented Jul 4, 2016

One unfortunate consequence is that redux-form worked "out of the box" with many third party input widgets, where you could just say

<Field name="color" component={DropdownList} data={colors}/>

Where as now, you have to pull the input props out for it with a:

const renderDropdownList = field => <DropdownList {...field.input}/>
...
<Field name="color" component={renderDropdownList} data={colors}/>

😢

@jimbolla
Copy link

jimbolla commented Jul 4, 2016

@erikras Seems like there's 3 possible behaviors:

  1. component = dom: should only get {...field.input}
  2. component = component that wants flat: should get {...field} {...field.input}
  3. component = component that wants nested: should get {...field}

You could handle this with extra props on <Field>. Ex:

  <Field name="name" component="input"> <!-- autodetects dom=true because string, same as: ->
  <Field name="name" component="input" dom>

  <Field name="color" component={DropdownList} data={colors} flat /> <!-- flat tells Field to spread field.input -->

@erikras
Copy link
Member Author

erikras commented Jul 4, 2016

@jimbolla Excellent thinking! I've already implemented 1 and 3, but I definitely see some value in a flat prop that would get around the field => <DropdownList {...field.input}/> silliness. Thanks for that.

@gaearon
Copy link
Contributor

gaearon commented Jul 4, 2016

flat and dom seem like very magic solutions to me.
They might make sense today in the context of “how do I get rid of this warning?”
I’m not sure they would make sense in a year for people reading the code using Redux Form.

One unfortunate consequence is that redux-form worked "out of the box" with many third party input widgets, where you could just say

This pattern is problematic and can lead to bugs. If third party DropDownComponent adds a field called error (new feature—minor per semver), your app would suddenly start passing Redux Form errors to it, and potentially crash or lead to unexpected behavior. This doesn’t seem like a pattern you’d want to encourage.

@jtbennett
Copy link

I think being explicit about the props that are passed is the way to go -- everything else in react/redux tends to be explicit in the same way. (A good thing, IMO.) Making those props available on field.input makes a lot of sense to me.

Before I found redux-form and I was starting to roll my own, an input sub-prop was what I came up with. So if I'm anything close to normal and a single piece of anecdotal evidence is worth anything, that's what I'd like to see. :)

@yanndinendal
Copy link

yanndinendal commented Jul 5, 2016

What about textareas or selects? For example, field.value is not valid for a textarea.

And even inputs don't have always the same props: checked only exists for some input types.

@padsbanger
Copy link

I getting the very same error , using Redux Form 5.2.5:

app.js:2483 Warning: Unknown propsinitialValue,autofill,onUpdate,valid,invalid,dirty,pristine,active,touched,visited,autofilledon <input> tag. Remove these props from the element. For details, see https://fb.me/react-unknown-prop

is there any way to get rid of this error, by downgrading to specific version of the library for example ?

@DimaRGB
Copy link

DimaRGB commented Jul 5, 2016

@padsbanger don't use react v15.2 for redux-form v5.

Or omit all this props from every fields.

@erikras
Copy link
Member Author

erikras commented Jul 5, 2016

What about textareas or selects? For example, field.value is not valid for a textarea.

And even inputs don't have always the same props: checked only exists for some input types.

@yanndinendal Those are covered by React.

@erikras
Copy link
Member Author

erikras commented Jul 5, 2016

Published fix in 6.0.0-rc.2.

@ericnograles
Copy link

ericnograles commented Jul 22, 2016

Sorry guys, I arrived super late to this party, but I wanted to bump @jasongonzales23's prior comment. My code is structured the same way as his with 6.0.0-rc3 (swapped out the {...whateverField} destructuring to {...whateverField.input} for a vanilla <input>) but I also got the same RSOD complaining about "no property input".

[Update]: Durr, I'm an idiot, I see the v6 migration guide and using the Field component. For posterity: http://redux-form.com/6.0.0-alpha.15/docs/MigrationGuide.md/

@jasongonzales23
Copy link

@ericnograles thanks! Somehow I missed the migration guide. Will implement this soon!

@MrSkinny
Copy link

MrSkinny commented Jul 23, 2016

Sorry I'm a bit new on this but I followed the migration v6 guide to try to resolve the Unknown properties warning. All warnings are now gone, but my form is behaving super strange -- basically, I can manually focus an input element, type one character and then it suddenly loses focus! I re-focus the input using the mouse and can type normally. When I tab to next field, the same de-focus happens. But after the second time of manually re-focusing the input, this never happens again and I can type and tab between fields as normal. 100% replicable. This is my component:

import React, { Component } from 'react';
import { reduxForm, Field } from 'redux-form';
import { createPost } from '../actions';

const renderFormcontrol = function (props, controlType) {
  const CustomTag = `${controlType}`;
  console.log(props);

  return(
    <div>
      <CustomTag {...props.input} />
      {props.touched &&
        props.error &&
      <span className="error">{props.error}</span>}
    </div>
  );
};

class PostsNew extends Component {
  render () {
    const { handleSubmit } = this.props;

    return(
      <div className="container">
        <form onSubmit={handleSubmit(props => createPost(props))}>
          <h3>Create Post</h3>
          <div className="form-group">
            <label>Title</label>
            <Field type="text" className="form-control" component={props => renderFormcontrol(props, 'input')} name="title" />
          </div>
          <div className="form-group">
            <label>Categories</label>
            <Field type="text" className="form-control" component={props => renderFormcontrol(props, 'input')} name="categories" />
          </div>
          <div className="form-group">
            <label>Content</label>
            <Field className="form-control" component={props => renderFormcontrol(props, 'textarea')} name="content" />
          </div>

          <button type="submit" className="btn btn-primary">Submit</button>
        </form>
      </div>
    );
  }
}

export default reduxForm(
  {
    form: 'PostsNew'
  },
  null,
  { createPost }
)(PostsNew);

@MrSkinny
Copy link

OK, I've confirmed it's happening because instead of using a renderInput function, I was allowing a dynamic element name to be passed so you could use the same function for rendering a textarea.

@raapperez
Copy link

The redux-form v6 looks great but I'm still with problems using third-party inputs. The following code throws the warning:
bundle.js?v=1:9310 Warning: Unknown props defaultCountry, active, asyncValidating, dirty, error, invalid, input, pristine, touched, valid, visited, autoFormat, onlyCountries, excludeCountries, isValid, flagsImagePath, onEnterKeyPress on tag. Remove these props from the element.

<Field name="phone" component={props => ( <ReactPhoneInput defaultCountry={'br'} {...props} required/> )} />

I know some of the wrong properties come from the ReactPhoneInput but others are still from redux-form.

@elyobo
Copy link

elyobo commented Jul 26, 2016

@erikras, as @oliverchoy asked, is there any plan to fix this in v5? As v6 is not yet out it would be nice to have a v5 fix.

@Strato
Copy link
Contributor

Strato commented Jul 28, 2016

v5 fix expected here too...

@notadamking
Copy link

+1 for fixing this in v5. #1444 shows why v5 is preferred over v6 currently

@realbugger
Copy link

Would love to have a v5 fix as well. Migrating to v6 isn't really feasible for the reason stated in #1444.

@elyobo
Copy link

elyobo commented Aug 10, 2016

Re: v5 props, @erikras has mentioned a workaround elsewhere, copied below. It'll silence the noise anyway.

const domOnlyProps = ({
  initialValue,
  autofill,
  onUpdate,
  valid,
  invalid,
  dirty,
  pristine,
  active,
  touched,
  visited,
  autofilled,
  ...domProps }) => domProps

@SOSANA
Copy link

SOSANA commented Aug 11, 2016

for v5 will the doc's be updated to reflect handling this issue? anyone have a v5 example to share of using this function to pass in our fields object?

@oliverchoy
Copy link

The workaround worked out perfectly. Thank you @elyobo for posting on this thread.

@elyobo
Copy link

elyobo commented Aug 11, 2016

@SOSANA it just picks out the extra fields that have meaning to redux-form and returns the rest, so you can just spread the result, e.g.

const { fields: { myfield } } = this.props;
return <input {...domOnlyProps(myfield)} />;

instead of

const { fields: { myfield } } = this.props;
return <input {...myfield} />;

@SOSANA
Copy link

SOSANA commented Aug 11, 2016

@elyobo thanks for quick reply! worked as expected

@yourfavorite
Copy link

yourfavorite commented Aug 23, 2016

@elyobo Thanks for sharing the fix on this. The warning is gone in console now, but eslint is throwing errors due to no-unused-vars on each of the properties within domOnlyProps. Any suggestions for fixing this aside from disabling no-unused-vars for the file?

@elyobo
Copy link

elyobo commented Aug 23, 2016

@yourfavorite I disable that warning for this function; the unused vars are deliberate in this case, using destructuring to pick out the props that redux form cares about so that we can pass back everything else.

My version looks like this -

/* eslint-disable no-unused-vars */                                             
// TODO: remove once upgraded to redux-form v6, which makes this redundant      
export const domOnlyProps = ({                                                  
  initialValue,                                                                 
  autofill,                                                                     
  onUpdate,                                                                     
  valid,                                                                        
  invalid,                                                                      
  dirty,                                                                        
  pristine,                                                                     
  active,                                                                       
  touched,                                                                      
  visited,                                                                      
  autofilled,                                                                   
  error,                                                                        
  ...domProps,                                                                  
}) => domProps;                                                                 
/* eslint-enable no-unused-vars */                                              

Herbertvc added a commit to Herbertvc/React_101 that referenced this issue Aug 24, 2016
@ArmorDarks
Copy link

Quite dumb proposal, but alternatively data- prefix could be used for such 3rd-party props. Yes, they will end up in DOM, but it might be even benefit in some cases.

@glcheetham
Copy link

Async validation documentation this documentation is not updated with the v6 way of doing things (props under input, meta)

@gustavohenke
Copy link
Collaborator

@glcheetham, you're looking at a way too old version of the docs! A lot of stuff changed since v6.0.0-rc.1.
http://redux-form.com/6.5.0/examples/asyncValidation/

gabrielpoca added a commit to gabrielpoca/meteor-redux-demo that referenced this issue Mar 29, 2017
This change solves issue
#3 that's caused
by redux-form/redux-form#1249 .
@lock
Copy link

lock bot commented Jun 1, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests