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

AccessorSlot and LambdaAccessorSlot should populate value in Property Descriptor #1601

Open
nabacg opened this issue Aug 30, 2024 · 3 comments
Labels
bug Issues considered a bug Ecma Incompatibility Issues about Rhino being incompatible with the EcmaScript spec PropertyDescriptors

Comments

@nabacg
Copy link
Contributor

nabacg commented Aug 30, 2024

Follow up on #1577 , AccessorSlot and LambdaAccessorSlot should add value attribute to their Property Descriptor when getter / setter are not set.

Use cases to where it matters:
When somePDInstance.value will return undefined in both cases, but these will display different behavior:

  • somePDInstance.hasOwnProperty/Object.hasOwn: false vs. true
  • Object.getOwnPropertyDescriptor: undefined vs. Object
  • Object.getOwnPropertyDescriptors/Object.keys/Object.entries: missing entry for value property PD vs including a PD for the value property
  • forOf/forIn loops won't include the missing value property

from, there is a good discussion in PR review with a lot more details.

How it works in Chrome / Node

> const o = {};
undefined

>Object.defineProperty(o, "b", {
		enumerable: true,
		configurable: true
});
{b: undefined}


> Object.getOwnPropertyDescriptor(o, "b")
{value: undefined, writable: false, enumerable: true, configurable: true}
@nabacg nabacg changed the title AccessorSlot and LambdaAccessorSlot should populate value in Property Descriptor [ PropertyDescriptors ] - AccessorSlot and LambdaAccessorSlot should populate value in Property Descriptor Aug 30, 2024
@p-bakker p-bakker added PropertyDescriptors bug Issues considered a bug Ecma Incompatibility Issues about Rhino being incompatible with the EcmaScript spec labels Aug 30, 2024
@p-bakker p-bakker changed the title [ PropertyDescriptors ] - AccessorSlot and LambdaAccessorSlot should populate value in Property Descriptor AccessorSlot and LambdaAccessorSlot should populate value in Property Descriptor Aug 30, 2024
@p-bakker
Copy link
Collaborator

p-bakker commented Sep 6, 2024

@nabacg think you might create a PR for this one, or even a larger PR fixing some of the other PropertyDescriptor related cases as well?

@tonygermano
Copy link
Contributor

I think by definition an Accessor Property,

  1. must have a Get or Set slot where at least one of them holds a reference to a Callable. The other can also be a Callable or undefined.
  2. must not have a value slot

In your example, that would create a Data Property, not an Accessor Property, because it has neither a get nor set method defined.

@tonygermano
Copy link
Contributor

tonygermano commented Sep 18, 2024

For example, also in Chrome (note that when I provide a get method, the descriptor has a property set with an undefined value, but no properties called value or writable because those belong to Data Properties):

>  const o = {}
<- undefined

>  Object.defineProperty(o, "b", {
    enumerable: true,
    configurable: true,
    get() { return "c" }
})
    
<- {}
>  Object.getOwnPropertyDescriptor(o, "b")
<- {set: undefined, enumerable: true, configurable: true, get: ƒ}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues considered a bug Ecma Incompatibility Issues about Rhino being incompatible with the EcmaScript spec PropertyDescriptors
Projects
None yet
Development

No branches or pull requests

3 participants