facebook / facebook/hermes

defineProperty proxy hook is not called for Array length

Open
#1,910 0 comments 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.