defineProperty proxy hook is not called for Array length
- Dominant language
- JavaScript
- Stars
- 11.3k
- Forks
- 859
- Avg merge
- 1h 30m
- Merged PRs (30d)
- 3
Description
## Bug Description
Hey, I'm back with more Proxy issues.
I encountered a bug in a RN application that works through a Proxy-based state manager. It turned out that the array length subscription wasn't working. A closer look revealed that Hermes wasn't calling the `defineProperty()` hook when the length changed, as stated in the specification.
Correct flow by spec for push (and one level proxying Proxy -> Array):
...
[Array.prototype.push](https://262.ecma-international.org/16.0/index.html#sec-array.prototype.push)
// item operations
[[[Proxy::Set]]\(length)](https://262.ecma-international.org/16.0/index.html#sec-proxy-object-internal-methods-and-internal-slots-set-p-v-receiver)
// calling proxy `set()` hook or...
[[[Ordinary::Set]]\(length, receiver=Proxy)](https://262.ecma-international.org/16.0/index.html#sec-ordinary-object-internal-methods-and-internal-slots-set-p-v-receiver)
[OrdinarySet\(length, receiver=Proxy)](https://262.ecma-international.org/16.0/index.html#sec-ordinaryset)
[OrdinarySetWithOwnDescriptor\(length, receiver=Proxy)](https://262.ecma-international.org/16.0/index.html#sec-ordinarysetwithowndescriptor)
[[[Proxy::DefineOwnProperty]]\(length)](https://262.ecma-international.org/16.0/index.html#sec-proxy-object-internal-methods-and-internal-slots-defineownproperty-p-desc)
// calling proxy `defineProperty()` hook or...
[[[Array::DefineOwnProperty]]\(length)](https://262.ecma-international.org/16.0/index.html#sec-ordinary-object-internal-methods-and-internal-slots-defineownproperty-p-desc)
[[[Array::ArraySetLength]]](https://262.ecma-international.org/16.0/index.html#sec-arraysetlength)
[OrdinaryDefineOwnProperty\(length)](https://262.ecma-international.org/16.0/index.html#sec-ordinarydefineownproperty)
...
- [x] I have run `gradle clean` and confirmed this bug does not occur with JSC
- [x] The issue is reproducible with the latest version of React Native.
Hermes git revision (if applicable): 896ee1e4377bc81823212a4abdef482d659948b1
## Steps To Reproduce
```js
let log = (...args) => typeof print === 'undefined' ? console.log(JSON.stringify(args)) : print(JSON.stringify(args))
let arr = new Proxy([], {
defineProperty(target, property, attributes) {
log('define', target, property, attributes)
return Reflect.defineProperty(target, property, attributes)
},
deleteProperty(target, p) {
log('del', target, p)
return Reflect.deleteProperty(target, p)
},
})
log('push')
arr.push('a')
log('push')
arr.push('b')
log('push')
arr.push('c')
log('pop')
arr.pop()
log('shift')
arr.shift()
log('length=')
arr.length = 1
```
An example of how hooks work in v8
```sh
node t.js
["push"]
["define",[],"0",{"value":"a","writable":true,"enumerable":true,"configurable":true}]
["define",["a"],"length",{"value":1}]
["push"]
["define",["a"],"1",{"value":"b","writable":true,"enumerable":true,"configurable":true}]
["define",["a","b"],"length",{"value":2}]
["push"]
["define",["a","b"],"2",{"value":"c","writable":true,"enumerable":true,"configurable":true}]
["define",["a","b","c"],"length",{"value":3}]
["pop"]
["del",["a","b","c"],"2"]
["define",["a","b",null],"length",{"value":2}]
["shift"]
["define",["a","b"],"0",{"value":"b"}]
["del",["b","b"],"1"]
["define",["b",null],"length",{"value":1}]
["length="]
["define",["b"],"length",{"value":1}]
```
And this is how it works in hermes static_h:
```sh
hermes t.js
["push"]
["define",[],"0",{"value":"a","writable":true,"enumerable":true,"configurable":true}]
["push"]
["define",["a"],"1",{"value":"b","writable":true,"enumerable":true,"configurable":true}]
["push"]
["define",["a","b"],"2",{"value":"c","writable":true,"enumerable":true,"configurable":true}]
["pop"]
["del",["a","b","c"],"2"]
["shift"]
["define",["a","b"],"0",{"value":"b"}]
["del",["b","b"],"1"]
["length="]
```
Contributor guide
Research direction
Start by running the supplied t.js reproduction with Hermes static_h and compare its proxy logs with the V8 output and the linked ECMAScript steps. Trace the array length path through Proxy::Set, OrdinarySet and ArraySetLength. Done means defineProperty() is invoked for array length changes as shown in the specification and reproduction.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100