fix: avoid isObject for internal use
#18
Conversation
src/object.ts
Outdated
| @@ -275,10 +281,10 @@ export const assign = <X extends Record<string | symbol | number, any>>( | |||
| override: X | |||
| ): X => { | |||
| if (!initial || !override) return initial ?? override ?? {} | |||
| const merged = { ...initial } | |||
| const merged = cloneObject(initial) | |||
There was a problem hiding this comment.
Point of discussion
This line changes the behavior of assign in two subtle ways:
- It copies non-enumerable properties into the new object.
- It copies the prototype into the new object, potentially calling a class constructor.
For these reasons, this is a breaking change, BUT the behavior of assign is so woefully underspecified that we could probably get away with publishing this in a minor version.
The question is, “what's the intuitive behavior?” One might argue it's whatever aligns with Object.assign, as this is basically a recursive version of that.
The best course of action is likely to implement a simpler variant of cloneObject that does { ...initial } unless an object created with Object.create(null) is found, in which case, a mixture of Object.create(null) & Object.assign is performed.
There was a problem hiding this comment.
After writing that out, the answer is clear that what I described in the last paragraph is how it should work.
aleclarson
force-pushed
the
fix/avoid-isObject
branch
from
8dc2db5 to
cf58a48
Compare
aleclarson
force-pushed
the
main
branch
3 times, most recently
from
4beb4ed to
c938c1b
Compare
aleclarson
force-pushed
the
fix/avoid-isObject
branch
2 times, most recently
from
79f2cad to
5e6bfd3
Compare
aleclarson
force-pushed
the
main
branch
9 times, most recently
from
3e61d98 to
47f126a
Compare
aleclarson
force-pushed
the
fix/avoid-isObject
branch
from
5e6bfd3 to
9dc1b70
Compare
* docs: add warning about `isObject` and `Object.create(null)` * fix: use isPlainObject internally * test: assign/keys with Object.create(null) * fix: preserve null prototype in assign result
Description
Using
isObjectinternally is hostile to any use case in need ofObject.create(null), so we'll useisPlainObjectinstead.Affected functions include:
assigncrushkeysWhat does this mean?
When an object created with
Object.create(null)is nested in the argument toassign,crush, orkeys, it will be treated as if it was created with{}. In the case ofassign, any time such an object is assigned to, the resulting clone of the object will have a null prototype as expected.Checklist
/docsdirectory) has been updatedResolves
Resolves sodiray/radash#409