-
-
Notifications
You must be signed in to change notification settings - Fork 148
Fix: when 'value' is an empty and has 'defaultValue', 'value' will be replace. #450
base: master
Are you sure you want to change the base?
Conversation
src/index.js
Outdated
@@ -90,7 +90,7 @@ options.vnode = vnode => { | |||
if (vnode.children) attrs.children = vnode.children; | |||
|
|||
if (attrs.defaultValue) { | |||
if (!attrs.value && attrs.value!==0) { | |||
if (!attrs.value && attrs.value!==0 && attrs.value!=='') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we used this check instead of all three?
if (attrs.value==null) {
attrs.value = attrs.defaultValue;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, only undefined
should be ignored.
if (attrs.defaultValue) {
if (attrs.value === undefined) {
attrs.value = attrs.defaultValue;
}
delete attrs.defaultValue;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, in
check will be most strictly, but, preact-compat will merge defaultValue to value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is React's behavior here? They usually differentiate between null
and undefined
, but I haven't tested this specific behavior. We should just use whatever they use to avoid an inconsistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also ran into problems with the preact-compat
implementation of defaultValue
.
In React, the defaultValue
prop will not set the value
prop on an element in general. In my observations, the value
prop remains undefined
. Changing the defaultValue
prop does not cause any further changes/renders on the component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The described behavior applies to the case of a select
.
No description provided.