Skip to content

refactor(value): align native type checks with quickjs-ng#748

Merged
buke merged 4 commits into
mainfrom
feat/value-pr2-native-type-checks
May 22, 2026
Merged

refactor(value): align native type checks with quickjs-ng#748
buke merged 4 commits into
mainfrom
feat/value-pr2-native-type-checks

Conversation

@buke
Copy link
Copy Markdown
Owner

@buke buke commented May 22, 2026

  • switch promise, typed array, and array buffer detection to native quickjs-ng APIs
  • add native collection, proxy, and immutable ArrayBuffer helpers with regression tests
  • cover value.go changes with focused tests and coverage validation

- switch promise, typed array, and array buffer detection to native quickjs-ng APIs
- add native collection, proxy, and immutable ArrayBuffer helpers with regression tests
- cover value.go changes with focused tests and coverage validation
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for Proxy objects, adds immutability controls for ArrayBuffers, and implements several new type predicates such as IsRegExp, IsMap, and IsProxy. It also refactors existing type checks and the ToUint8Array method to use native QuickJS functions for better efficiency. A critical bug was identified in the NewProxy implementation where target and handler references are consumed by the underlying C function without being incremented, which would lead to double-free errors during garbage collection.

Comment thread context.go
@codecov
Copy link
Copy Markdown

codecov Bot commented May 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (d586cac) to head (44fa3ca).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #748   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines         3726      3766   +40     
=========================================
+ Hits          3726      3766   +40     
Files with missing lines Coverage Δ
context.go 100.00% <100.00%> (ø)
value.go 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d586cac...44fa3ca. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- add detached Uint8Array coverage for the native JS_GetUint8Array failure path
- add nil-context proxy coverage for the Context.NewProxy guard branch
@buke
Copy link
Copy Markdown
Owner Author

buke commented May 22, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces several new methods and predicates to the Context and Value types, including support for Proxies (NewProxy, ProxyTarget, ProxyHandler), immutable ArrayBuffers, and various JavaScript built-in types like Map, Set, and RegExp. It also refactors TypedArray detection and ToUint8Array to use native QuickJS functions. Feedback was provided to ensure ToUint8Array checks for a valid context to prevent panics, to correct the logic in typedArrayType regarding JS_TYPED_ARRAY_NONE, and to maintain consistency in boolean conversions.

Comment thread value.go
Comment thread value.go
Comment thread value.go
buke added 2 commits May 22, 2026 18:43
- add detached ArrayBuffer coverage for the native ToByteArray failure branch
- add detached TypedArray coverage for native conversion error branches
- replace fake typed-array error coverage with detached real buffer/view cases
- cover marshal ArrayBuffer and TypedArray decode failure branches after native checks
@buke buke merged commit 1dbcb3c into main May 22, 2026
8 checks passed
@buke buke deleted the feat/value-pr2-native-type-checks branch May 22, 2026 11:07
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