Skip to content

fix defineProperty() proxy hook for array length#1911

Open
farwayer wants to merge 2 commits intofacebook:static_hfrom
farwayer:arr-length-define-hook
Open

fix defineProperty() proxy hook for array length#1911
farwayer wants to merge 2 commits intofacebook:static_hfrom
farwayer:arr-length-define-hook

Conversation

@farwayer
Copy link

Summary

Fixes #1910

internalSetter() should be called in JSObject::putNamedWithReceiver_RJS() only if target object is the same as receiver.
The lack of check resulted in the Proxy::defineOwnProperty() call being skipped for the array's internal length property. This, in turn, resulted in the Proxy's defineProperty() hook being ignored.

Correct flow by spec for push (and one level proxying Proxy -> Array):
...
Array.prototype.push
// item operations
[[Proxy::Set]](length)
// calling proxy set() hook or...
[[Ordinary::Set]](length, receiver=Proxy)
OrdinarySet(length, receiver=Proxy)
OrdinarySetWithOwnDescriptor(length, receiver=Proxy)
[[Proxy::DefineOwnProperty]](length)
// calling proxy defineProperty() hook or...
[[Array::DefineOwnProperty]](length)
[[Array::ArraySetLength]]
OrdinaryDefineOwnProperty(length)
...

Test Plan

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

Result before fix:

["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="]

After:

["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}]

@meta-cla meta-cla bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

defineProperty proxy hook is not called for Array length

1 participant