justmoon/node-extend

Support for symbols as keys

homer0 opened this issue · 6 comments

When you merge objects that use symbols as keys, they get removed.

I did some research and testing and I believe the issue is here:

for (name in options) {

The for ... in ignores the symbols, so I tried changing it a little bit:

- for (name in options) {
+ const names = [
+     ...Object.getOwnPropertySymbols(options),
+     ...Object.keys(options)
+ ];
+ for (name of names) {

And now it works.

The spread can be replaced with concat, but getOwnPropertySymbols pretty much leaves IE out and I don't know which are the browser/engines the library should support, that's why I didn't send a PR.

Let me know what you think.
Thanks.

This package is a port of jquery's extend method. Does that support symbols?

@ljharb we both know that it doesn't.

I never imagined this project was limited to what jQuery does or doesn't support. More so seeing that #35 was considered at one point.

Can you add something on the README or a CONTRIBUTING file?

Sorry for waisting your time!

The readme already says it’s a port of jquery’s extend.

@ljharb yes, but that doesn't mean that you won't make improvements or accept suggestions; those are two different things.

That’s true, but if you want to include symbols, you can use object spread or Object.assign already?

@ljharb yep, but not for deep merge :P. No worries, I'll code something, is not that complicated :D.