Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
[React 15.2.0] Additional props warning #1249
Comments
|
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 InputWrapperAnd to make it the default component of Field.defaultProps = {
component: InputWrapper,
} |
ooflorent
added
the
discussion
label
Jul 1, 2016
|
Another workaround is to instrument |
|
Results of first test:
It's amusing how accustomed whoever wrote that warning message is with GitHub Markdown that they used backquotes. |
|
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... |
|
Maybe |
tylergoodman
commented
Jul 1, 2016
|
Is nesting the input-specific props an option? eg.
|
|
@tylergoodman That is an option, yes. |
jambonrose
referenced this issue
in Hacker0x01/react-datepicker
Jul 2, 2016
Closed
React 15.2 Unkown Prop Warning #517
|
@tylergoodman’s suggestion looks cleanest. |
|
@gaearon Stinks of "breaking change", though..... |
|
You could..
|
|
Ooh, for a moment there, I thought I might be clever and make the |
|
Nah, smart tricks are only going to make things worse. Please fix the underlying issue. |
|
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 <Field name="username" component={renderField} type="text" placeholder="Username"/>
// Those guys ---> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ |
|
Yea, IMO it’s the most elegant and clean solution. |
|
One unfortunate consequence is that <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}/> |
This was referenced Jul 4, 2016
jimbolla
commented
Jul 4, 2016
•
|
@erikras Seems like there's 3 possible behaviors:
You could handle this with extra props on
|
|
@jimbolla Excellent thinking! I've already implemented 1 and 3, but I definitely see some value in a |
|
This pattern is problematic and can lead to bugs. If third party |
jtbennett
commented
Jul 4, 2016
|
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
commented
Jul 5, 2016
•
|
What about textareas or selects? For example, And even |
padsbanger
commented
Jul 5, 2016
|
I getting the very same error , using Redux Form 5.2.5:
is there any way to get rid of this error, by downgrading to specific version of the library for example ? |
DimaRGB
commented
Jul 5, 2016
•
|
@padsbanger don't use react v15.2 for redux-form v5. Or omit all this props from every fields. |
|
|
Published fix in |
erikras
closed this
Jul 5, 2016
emborg
commented
Jul 6, 2016
•
|
in v6 rc2 have error in v6 rc1 unknown props was picked with (as suggested above) |
frost555
commented
Jul 6, 2016
•
|
Found the issue that user defined props are transfered on props.input.
How can i workaround it? As you can see - there is no need to pass myOnBlur prop to input DOM element.
|
|
@frost555 No, that is as designed. It must be that way to get things like |
frost555
commented
Jul 6, 2016
•
|
Sorry for editing my message. |
|
@frost555 Maybe. I'd be open to hearing arguments for and against. |
|
I'm having hard time migrating from RC1->RC2 because many of my HOCs assumes that the props passed to the Alternative to @frost555's proposal would be to make it other way around: <Field name="field1" containerProps={{myOnBlur: "goes to props"}} xyz="goes to props.input" ...This would be compatible with RC2. |
joeshub
referenced this issue
in gajus/react-css-modules
Jul 6, 2016
Closed
react Warning: Unknown prop `styleName` #153
q42jaap
commented
Jul 7, 2016
|
Hi @erikras, I think the docs under |
q42jaap
commented
Jul 7, 2016
|
|
jason-whitted
commented
Jul 7, 2016
|
Like @epeli, I'm also having a difficult time migrating from RC1->RC2. Many of my components can be used independent of a It seems we need some way to specify props that should be relayed (not placed in |
frost555
commented
Jul 7, 2016
|
Yep, i agree. "containerProps" would be OK for me too. |
babsonmatt
commented
Jul 7, 2016
|
Is the main reason for "containerProps" just because it's ugly to have that stuff thrown onto the input? It doens't throw the 15.2 warning right? |
Yes. If you previously had an input renderer that was like: renderWidget = ({ mySpecialLabel, ...rest }) =>
<div>
<label>{mySpecialLabel}</label>
<input {...rest}/>
</div>
// called with
<Field name="whatever" compoenent={renderWidget} mySpecialLabel="Used to go into root"/>It now has to be: renderWidget = ({ input: { mySpecialLabel, ...rest } }) =>
// ^^^^^^^^------ not that big a deal, if you ask me
<div>
<label>{mySpecialLabel}</label>
<input {...rest}/>
</div>
// called with
<Field name="whatever" compoenent={renderWidget} mySpecialLabel="Now goes into input"/>You just have to learn:
It's # 3 that people are complaining about, and I'm certain that should be the default behavior, but I'm not really convinced that adding a special prop name to force values into the root solves much. I agree that it would be nice if only input-like props (e.g. |
jason-whitted
commented
Jul 7, 2016
|
The problem is not that it "looks ugly". The problem is that Field is making assumptions about the props and tampering with them. I understand completely why the input-related props were moved to For example, a |
babsonmatt
commented
Jul 7, 2016
|
So now we lose prop type validation on the individual properties that we're now receiving via input prop on the underlying component right? |
input: PropTypes.shape({
whatever: PropTypes.object,
whatever2: PropTypes.number
})? |
|
@jason-whitted Yes, I agree with all of that. See the @babsonmatt Darn, @gaearon beat me to it. |
babsonmatt
commented
Jul 7, 2016
•
jason-whitted
commented
Jul 7, 2016
•
|
I still don't think I've done a good example of explaining the issue. Here is an example of a FormGroup being used without being nested in a Field. <FormGroup label="Example" content={props => <b>Example</b>} />This would render as: <div className="form-group">
<label>Example</label>
<b>Example</b>
</div>And here is an example being nested in a field. <Field name="firstName" component={FormGroup} label="Example" content={TextField} />
|
babsonmatt
commented
Jul 7, 2016
|
@jason-whitted so it couples the form component with the redux form way of doing things |
|
@jason-whitted You're going to need wrapper components. See #1285 (comment). |
babsonmatt
commented
Jul 7, 2016
|
@erikras thanks |
|
for those upgrading, |
|
Oh crap. Sorry about that, @jayphelps. I will modify the release notes. |
jason-whitted
commented
Jul 7, 2016
Your proposal is to create a wrapper component to flatten the props and then another wrapper component to rebuild the
const flattenInputProps = Component => ({ input, ...other }) => <Component {...other} {...input} />;
const unflattenInputProps = Component => ({
autofilled,
dirty,
error,
initialValue,
invalid,
name,
pristine,
touched,
valid,
...other
}) => <Component autofilled={autofilled}
dirty={dirty}
error={error}
initialValue={initialValue}
invalid={invalid}
name={name}
pristine={pristine}
touched={touched}
valid={valid}
input={...other} />;
<Field name="firstName"
component={flattenInputProps(FormGroup)}
label="Example"
content={unflattenInputProps(TextField)} /> |
|
@jason-whitted Are |
jason-whitted
commented
Jul 7, 2016
|
// NOTE: FormGroup has no dependency on Field (or any knowledge of redux-form for that matter)
const FormGroup = ({ label, content: Content, ...other }) => (
<div className="form-control">
<label>{label}</label>
<Content {...other} />
</div>
);
// NOTE: TextField on the other hand is aware of redux-form and is trying to pass on the input props.
const TextField = ({ input, ...other }) => <input type="text" {...input} />;<Field name="firstName" component={FormGroup} label="Example" content={TextField} /> |
jason-whitted
commented
Jul 7, 2016
•
|
@erikras I created a repository with an example of the issue. |
|
Just wanted to chime in with a few thoughts..This has been a very non-trivial change for us since we depended heavily on custom components, so now they all require wrappers. Instead of doing that, we created a helper to do it for us since it was so common. import { Field } from 'redux-form';
import { pick } from 'lodash';
export const createField = (Component, { pick: pickKeys } = {}) => {
const Wrapped = ({ input, ...rest }) => {
let props = { ...input, ...rest };
if (pickKeys && pickKeys.length) {
props = pick(props, pickKeys);
}
return (
<Component {...props}>{input.children}</Component>
);
};
return props => ( // eslint-disable-line react/no-multi-comp
<Field {...props} component={Wrapped}>
{props.children}
</Field>
);
};You can then use it like so // Let's assume you have an Autocomplete component
// that accepts these props
Autocomplete.propTypes = {
onChange: PropTypes.func,
value: PropTypes.string,
options: PropTypes.arrayOf(PropTypes.string)
};
// Must do this outside of the render() because
// it must be stable, not changing between renders
const AutocompleteField = createField(Autocomplete);
const Example = () => (
<div>
<AutocompleteField
name="whatever"
options={['first', 'second', 'third']}
/>
</div>
);If you use such a thing, remember that by default you're now providing a bunch of excess props to to the component. If this is an issue, you can pick which ones you want: const CheckboxField = createField(Checkbox, { pick: ['label', 'onChange', 'value'] });Using this also means you don't need a separate file to satisfy the eslint rule no-multi-comp (disallowing multiple components per file, which is a good practice but in this case very awkward) |
jason-whitted
commented
Jul 8, 2016
•
|
I created a wrapper component that fixes my issue. It can also be used to fix any similar prop issue in the future. Higher Order Component/**
* HOC that remaps properties.
* @param {function} map - The remap function. (oldProps) => newProps
*/
const remapProps = map => Component => props => <Component {...map(props)} />;Exampleconst remap = ({ input: { label, content, ...input }, ...other }) =>
({ label, content, input, ...other });
const RemappedFormGroup = remapProps(remap)(FormGroup);
<Field name="firstName"
component={RemappedFormGroup}
label="Example"
content={TextField} />The end result is precisely what I was looking for. The It might be a nice courtesy to include the HOC in redux-form since it has such a small footprint. The example repository has been updated. |
|
@jason-whitted Looks very similar to |
|
I guess if you don't want to |
jason-whitted
commented
Jul 8, 2016
tylergoodman
commented
Jul 9, 2016
•
|
@erikras Regarding The big deal with this is that now you can't use renderWidget = ({ input: { mySpecialLabel, ...rest } }) =>
// ^^^^^^^^------ kind of a big deal, if you ask me
<div>
<label>{mySpecialLabel}</label>
<input {...rest}/>
</div>
// called with
<Field name="whatever" compoenent={renderWidget} mySpecialLabel="Now goes into input"/>
renderWidget.defaultProps = {
input: {} // oh no
};In any case I support the idea of an option that decides how props are merged |
neptunian
referenced this issue
in jossmac/react-images
Jul 10, 2016
Closed
Fix js warnings when using React 15.2 #63
oliverchoy
commented
Jul 10, 2016
|
@erikras are you going to port the fixes to v5? If not, what is the recommended react version to be used (anything < 15.2.0)? |
suhaotian
commented
Jul 11, 2016
•
|
This change will broken complex form component exist. Please don't do it. // In your component
const {active, dirty, error, invalid, onUpdate, pristine, touched, valid, visited, asyncValidating, autofocus, ...rest} = this.props
// your props in rest
const { el, type, value, onChange, ...other} = rest |
kristian-puccio
commented
Jul 12, 2016
|
just to pipe in with my opinion on the extra props being added to input it does make it hard to validate using flow. I am using a global redux-form definition that declares InputProps to be:
I generally do this to specify the props on my component:
If the extra props get added to input I'd have to override the whole thing rather than just extend it. Maybe a typescript person could comment about how this would work for them? |
kristian-puccio
commented
Jul 12, 2016
|
The other thing that I'm finding as I update my app to use v6 is that the list of props that I have to destructure out of the input is growing. This isn't really an issue if you use the props you extract in that file but I have components that I just pass props onto to draw stuff, like errors or labels etc. So I'm finding that I have to do a lot of // eslint-disable which I find quite uncomfortable as it's a potential source of errors. |
tylergoodman
commented
Jul 12, 2016
•
|
@kristian-puccio in Typescript you could do something similar to what you've done in flow interface InputProps<T> {
active: boolean;
touched: boolean;
error?: string;
input: {
name: string;
placeholder?: string;
label?: string;
value?: string | number;
type?: string;
} & T
}
interface ComponentProps {
extra: string;
}
type MyInputComponent = InputProps<ComponentProps>;
var a: MyInputComponent = {
active: true,
touched: true,
input: {
name: 'a',
extra: 'stuff',
},
}; |
kristian-puccio
commented
Jul 12, 2016
|
Ok that is nice. Cheers for that! On 12 July 2016 at 11:58, Tyler Goodman notifications@github.com wrote:
|
added a commit
to jvikstedt/jnotes_old
that referenced
this issue
Jul 13, 2016
jasongonzales23
commented
Jul 14, 2016
|
For some reason, using vanilla Here is what my inputs looked like before:
So the way I interpreted your instructions, I thought I had to modify them like so:
But that throws an error. Something to the effect of 'no property input...' Thanks! |
This was referenced Jul 14, 2016
added a commit
to Automattic/delphin
that referenced
this issue
Jul 18, 2016
This was referenced Jul 19, 2016
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 [Update]: Durr, I'm an idiot, I see the v6 migration guide and using the |
jasongonzales23
commented
Jul 23, 2016
|
@ericnograles thanks! Somehow I missed the migration guide. Will implement this soon! |
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:
|
MrSkinny
commented
Jul 23, 2016
|
OK, I've confirmed it's happening because instead of using a |
raapperez
commented
Jul 26, 2016
|
The redux-form v6 looks great but I'm still with problems using third-party inputs. The following code throws the warning:
I know some of the wrong properties come from the ReactPhoneInput but others are still from redux-form. |
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. |
|
v5 fix expected here too... |
tbash
referenced this issue
in facebookincubator/create-react-app
Aug 2, 2016
Closed
redux-form and create-react-app not working #325
adamjking3
commented
Aug 3, 2016
|
+1 for fixing this in v5. #1444 shows why v5 is preferred over v6 currently |
realbugger
commented
Aug 10, 2016
|
Would love to have a v5 fix as well. Migrating to v6 isn't really feasible for the reason stated in #1444. |
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
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
commented
Aug 11, 2016
|
The workaround worked out perfectly. Thank you @elyobo for posting on this thread. |
elyobo
commented
Aug 11, 2016
|
@SOSANA it just picks out the extra fields that have meaning to const { fields: { myfield } } = this.props;
return <input {...domOnlyProps(myfield)} />;instead of const { fields: { myfield } } = this.props;
return <input {...myfield} />; |
SOSANA
commented
Aug 11, 2016
|
@elyobo thanks for quick reply! worked as expected |
gnujoow
referenced this issue
in pjhjohn/jsonplaceholder-client
Aug 18, 2016
Closed
form validation #26
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
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 */ |
added a commit
to Herbertvc/React_101
that referenced
this issue
Aug 24, 2016
pjhjohn
referenced this issue
in pjhjohn/jsonplaceholder-client
Aug 27, 2016
Closed
Entering /posts/new causes warning #35
ArmorDarks
commented
Sep 10, 2016
|
Quite dumb proposal, but alternatively |
speedDeveloper
referenced this issue
in erikras/redux-form-material-ui
Dec 30, 2016
Closed
Console Warning: Unknown prop `validate` on <input> tag. Remove this prop from the element. #78
glcheetham
commented
Mar 3, 2017
|
Async validation documentation this documentation is not updated with the v6 way of doing things (props under |
|
@glcheetham, you're looking at a way too old version of the docs! A lot of stuff changed since v6.0.0-rc.1. |
erikras commentedJun 30, 2016
It will be interesting to see, when React
15.2.0is released, how many of these warningsredux-formgenerates. More info and discussion here from the page the warning links to:Props like
valid,invalid,dirty, andpristinemight cause warnings for cases where someone is just destructuring the field object into an input, i.e.<input type="text" {...firstName}/>.