Unconfigured props will be overwritten if a new undefined value is not passed in #376
Conversation
diegonvs
force-pushed
the
jsx-unconfigured-props
branch
from
3361a0e to
978e5ca
Compare
diegonvs
changed the title
|
depends on #377 |
diegonvs
changed the title
|
I'm working in tests for this use case. |
diegonvs
force-pushed
the
jsx-unconfigured-props
branch
from
03b6b4f to
f9d3461
Compare
diegonvs
changed the title
There was a problem hiding this comment.
Hey @diegonvs, this looks good! I think it can be simplified a little bit.
We probably don't need to keep array.dedupe moving forward, as it's pretty straightforward to do it using a Set.
I'm pinging @bryceosterhaus and @mthadley to validate this as well.
Thanks! :)
packages/metal/src/array/array.js
Outdated
| deduped.push(arr[i]); | ||
| } | ||
| } | ||
| return deduped; |
There was a problem hiding this comment.
Hey @diegonvs, I think this all could be simply...
return arr.filter(
(elem, pos) => {
return arr.indexOf(elem) === pos;
}
);There was a problem hiding this comment.
https://jsperf.com/filter-vs-for-to-dedupe-an-array
Using for is more performatic
There was a problem hiding this comment.
That's a premature optimization. We can get there if needed :)
There was a problem hiding this comment.
In any case, since we're not going to need it for now, let's simply remove it
| let keys = Object.keys(data); | ||
| let keys = array.dedupe( | ||
| Object.keys(data).concat(Object.keys(component.props)) | ||
| ); |
There was a problem hiding this comment.
I think this can be simplified to:
const keys = new Set([...Object.keys(data), ...Object.keys(component.props]);
keys.forEach(
key => {
...
}
);There was a problem hiding this comment.
Sounds good, I'll use it.
| @@ -411,6 +411,83 @@ describe('JSXComponent', function() { | |||
| assert.strictEqual(component.element, child.element); | |||
| }); | |||
|
|
|||
| it('should not overwrite unconfigured props if a new undefined value is not passed', function( | |||
There was a problem hiding this comment.
@bryceosterhaus, @mthadley, mind taking a look to see if this matches your expectations?
|
@jbalsas , Could you rerun travis? |
diegonvs
force-pushed
the
jsx-unconfigured-props
branch
from
7acc6a5 to
a1cbde4
Compare
…ster on github | Fixes metal#374
diegonvs
force-pushed
the
jsx-unconfigured-props
branch
from
184e9b9 to
917503c
Compare
|
Thanks @diegonvs! I'm merging this and plan to release all we've been accumulating some time next week. |
No description provided.