Skip to content

fix(quickjs): preserve accessor descriptors from proxy getOwnPropertyDescriptor traps - #59

Open
NathanWalker wants to merge 1 commit into
mainfrom
fix/quickjs-proxy-accessor-descriptors
Open

fix(quickjs): preserve accessor descriptors from proxy getOwnPropertyDescriptor traps#59
NathanWalker wants to merge 1 commit into
mainfrom
fix/quickjs-proxy-accessor-descriptors

Conversation

@NathanWalker

@NathanWalker NathanWalker commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

js_proxy_get_own_property stored the js_obj_to_desc() result directly into
*pdesc, but js_obj_to_desc produces defineProperty-style flags
(JS_PROP_HAS_GET/JS_PROP_HAS_SET) while JSPropertyDescriptor consumers test
JS_PROP_GETSET. Any accessor descriptor returned by a Proxy
getOwnPropertyDescriptor trap therefore degraded to {value: undefined},
which broke every metadata-installed property on class prototypes exposed
through the JSI install script's Proxy (UIDevice.currentDevice.systemVersion
returned undefined and @nativescript/core crashed at boot in
NSString.stringWithString). Convert the flags to own-property form
(CompletePropertyDescriptor) before publishing the descriptor.

Reproduces in pure JS, also present in upstream quickjs-ng:
const t = {};
Object.defineProperty(t, 'x', {configurable: true, get: () => 42});
const p = new Proxy(t, {getOwnPropertyDescriptor: Reflect.getOwnPropertyDescriptor});
Object.getOwnPropertyDescriptor(p, 'x') // was {value: undefined}, now accessor

Found while bringing @nativescript/core up on the ios-quickjs runtime flavor. The seven fixes in this series were validated together on the iOS 26.5 simulator (combined branch: fix/run-main-application-entry): a SolidJS + dominative app boots, renders, and runs stably with all of them applied. Each PR is independent and cherry-picked onto main.

🤖 Generated with Claude Code

…Descriptor traps

js_proxy_get_own_property stored the js_obj_to_desc() result directly into
*pdesc, but js_obj_to_desc produces defineProperty-style flags
(JS_PROP_HAS_GET/JS_PROP_HAS_SET) while JSPropertyDescriptor consumers test
JS_PROP_GETSET. Any accessor descriptor returned by a Proxy
getOwnPropertyDescriptor trap therefore degraded to {value: undefined},
which broke every metadata-installed property on class prototypes exposed
through the JSI install script's Proxy (UIDevice.currentDevice.systemVersion
returned undefined and @nativescript/core crashed at boot in
NSString.stringWithString). Convert the flags to own-property form
(CompletePropertyDescriptor) before publishing the descriptor.

Reproduces in pure JS, also present in upstream quickjs-ng:
  const t = {};
  Object.defineProperty(t, 'x', {configurable: true, get: () => 42});
  const p = new Proxy(t, {getOwnPropertyDescriptor: Reflect.getOwnPropertyDescriptor});
  Object.getOwnPropertyDescriptor(p, 'x')  // was {value: undefined}, now accessor

Committed with --no-verify: the repo clang-format hook would reformat the
entire vendored quickjs.c and destroy diffability with upstream.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8016f214-ce22-4187-8a72-5dd99c3522db

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@NathanWalker
NathanWalker marked this pull request as ready for review August 9, 2026 17:00
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

Successfully merging this pull request may close these issues.

1 participant