Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Follow behavior on destructuring for edge cases #5

Open
Alexsey opened this issue Jul 14, 2022 · 4 comments
Open

Follow behavior on destructuring for edge cases #5

Alexsey opened this issue Jul 14, 2022 · 4 comments

Comments

@Alexsey
Copy link

Alexsey commented Jul 14, 2022

There is a bunch of open questions like how to treat prototype properties, non-enumerable properties, __proto__, getters/setters etc.

It seems that following already existing rules for destructuring wherever possible should decrease the complexity of the proposal, provide intuitive UX, feet most of the use cases, and reduce the discussions. Consider open questions from the FAQ:
Item "1. When it comes to the prototype chain ..." can be removed because

Object.pick({a : 1}, ['toString']); // => {toString: f}
// is already equivalent to
const { toString } = { a: 1 }

Object.omit({a : 1}, ['toString']).toString; // => ƒ toString() { [native code] }
// is already equivalent to
const { toString, ...res } = { a: 1 }

Object.pick({}, ['__proto__']); // => {__proto__: {...}}
// is already equivalent to
const { __proto__ } = {}
const res = { __proto__ }

Object.omit({}, ['__proto__']).__proto__; // => {...}
// is already equivalent to
const { __proto__, ...res } = {}
res.__proto__

Item "2. What is the type of the returned value?" can be removed because

Object.pick([]); // => {} 
Object.omit([]); // => {}
Object.pick(new Map()); // => {} 
Object.omit(new Map()); // => {}
// is already equivalent to
const { ...res } = []
const { ...res } = new Map()
const { ...res } = new Set()

Item "3. How to handle Symbol?" can be removed because

Object.pick([], [Symbol.iterator]); // => {Symbol(Symbol.iterator): f}
// is already equivalent to
const { [Symbol.iterator]: iter } = []
const res = { [Symbol.iterator]: iter }

Object.omit([], [Symbol.iterator]); // => {}, plain objects
// is already equivalent to
const { [Symbol.iterator]: iter, ...res } = []
Symbol.iterator in res // false

const symbol = Symbol('key');
Object.omit({a : 1, [symbol]: 2}, [symbol]); // => {a : 1}
// is already equivalent to
const symbol = Symbol('key');
const { [symbol]: _, ...res } = {a : 1, [symbol]: 2}

Object.prototype[symbol] = 'test'; // override prototype
Object.pick({}, [symbol]); // => {Symbol(key): "test"}, pick off from the prototype
Object.omit({}, [symbol])[symbol]; // => "test", cannot omit properties from the prototype
// is already equivalent to
const { [symbol]: sym, ...res } = {}
sym // 'test'
res[symbol] // 'test'

Item "4. If some properties of an object are not accessible like throwing an error..." can be removed because

Object.pick(Object.defineProperty({}, 'key', {
   get() { throw new Error() }
}), ['key']);
// is already equivalent to
const o = Object.defineProperty({}, 'key', {
  get() { throw new Error('custom') }
})

try {
  const { key } = o
} catch (e) {
  e.message // 'custom'
}
@ljharb
Copy link
Member

ljharb commented Jul 14, 2022

I don't agree with Object.omit({}, [symbol])[symbol]; // => "test", cannot omit properties from the prototype - omit should work like destructuring with rest syntax. In other words, the new object copies ONLY own properties, so the prototype itself is completely lot.

@Alexsey
Copy link
Author

Alexsey commented Jul 14, 2022

It will work like destructuring because initially symbol was assigned to Object.prototype. The example here is quite exotic, but the behavior is correct. The only reasonable way to behave differently would be to return an object without prototype, and this is not how destructuring is working

@ljharb
Copy link
Member

ljharb commented Jul 14, 2022

The semantics of const rest = Object.omit(obj, [symbol]) should be precisely identical to const { [symbol]: _, ...rest } = obj except that the unneeded _ binding won't be created.

@aleen42
Copy link
Collaborator

aleen42 commented Jul 15, 2022

So is it necessary to describe these 4 situations in the draft, which should be consistent with destructuring? Or just remove them at all?

aleen42 added a commit to aleen42/proposal-object-pick-or-omit that referenced this issue Jul 15, 2022
aleen42 added a commit to aleen42/proposal-object-pick-or-omit that referenced this issue Jul 15, 2022
aleen42 added a commit to aleen42/proposal-object-pick-or-omit that referenced this issue May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants