Skip to content

Comments

perf(napi/parser): do not remove extraneous options on JS side#16447

Merged
graphite-app[bot] merged 1 commit intomainfrom
12-03-perf_napi_parser_do_not_remove_extraneous_options_on_js_side
Dec 3, 2025
Merged

perf(napi/parser): do not remove extraneous options on JS side#16447
graphite-app[bot] merged 1 commit intomainfrom
12-03-perf_napi_parser_do_not_remove_extraneous_options_on_js_side

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Dec 3, 2025

Similar to #16442.

There's no need to remove the experimental* properties from options object on JS side. They'll be ignored by NAPI-RS on Rust side anyway, so creating a new object on JS side is pointless overhead.

@github-actions github-actions bot added A-parser Area - Parser C-performance Category - Solution not expected to change functional behavior, only performance labels Dec 3, 2025
Copy link
Member Author


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@overlookmotel overlookmotel marked this pull request as ready for review December 3, 2025 14:34
Copilot AI review requested due to automatic review settings December 3, 2025 14:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes unnecessary object destructuring in the NAPI parser's raw transfer code to improve performance. The destructuring was stripping out experimentalLazy and experimentalRawTransfer properties from the options object before passing it to Rust bindings, but these properties are automatically ignored by NAPI-RS since they're not defined in the Rust ParserOptions struct.

Key changes:

  • Eliminated redundant object creation and property extraction on the JavaScript side
  • Simplified code by directly passing options without filtering

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
napi/parser/src-js/raw-transfer/lazy.js Removed destructuring that stripped experimentalLazy from options in parseSyncLazy and parse functions
napi/parser/src-js/raw-transfer/eager.js Removed destructuring that stripped experimentalRawTransfer from options in parseSyncRaw and parse functions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@overlookmotel overlookmotel self-assigned this Dec 3, 2025
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Dec 3, 2025
Copy link
Member Author

overlookmotel commented Dec 3, 2025

Merge activity

graphite-app bot pushed a commit that referenced this pull request Dec 3, 2025
Similar to #16442.

There's no need to remove the `experimental*` properties from `options` object on JS side. They'll be ignored by NAPI-RS on Rust side anyway, so creating a new object on JS side is pointless overhead.
@graphite-app graphite-app bot force-pushed the 12-03-perf_napi_parser_do_not_remove_extraneous_options_on_js_side branch from 98c53a9 to 35115a7 Compare December 3, 2025 14:46
Similar to #16442.

There's no need to remove the `experimental*` properties from `options` object on JS side. They'll be ignored by NAPI-RS on Rust side anyway, so creating a new object on JS side is pointless overhead.
@graphite-app graphite-app bot force-pushed the 12-03-perf_napi_parser_do_not_remove_extraneous_options_on_js_side branch from 35115a7 to 790beeb Compare December 3, 2025 14:47
@graphite-app graphite-app bot merged commit 790beeb into main Dec 3, 2025
18 checks passed
@graphite-app graphite-app bot deleted the 12-03-perf_napi_parser_do_not_remove_extraneous_options_on_js_side branch December 3, 2025 14:53
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Dec 3, 2025
overlookmotel pushed a commit that referenced this pull request Dec 8, 2025
### 💥 BREAKING CHANGES

- 083fea9 napi/parser: [**BREAKING**] Represent empty optional fields on
JS side as `null` (#16411) (overlookmotel)

### 🚀 Features

- 7a2afee parser: Add TS1174 error for classes extending multiple base
classes (#15993) (sapphi-red)
- da87812 semantic: Add TS2309 error for export assignment with other
exports (#15992) (sapphi-red)
- d6d2bcd minifier: Remove unused function calls that are marked by
`manual_pure_functions` (#16534) (sapphi-red)
- c90f053 minifier: Support `.` separated values for
`compress.treeshake.manualPureFunctions` (#16529) (sapphi-red)
- a607cc4 codegen: Preserve comments between CatchClause's param and
body (#16167) (copilot-swe-agent)
- 8c10694 semantic: Expose get_comment_at method (#16439) (camc314)
- 3981e7a ast: Add get_comment_at to lookup a comment by span (#16438)
(camc314)

### 🐛 Bug Fixes

- 699406a napi/parser: Move `ExportEntry::module_request` field to first
(#16412) (overlookmotel)
- 12bd794 napi/parser: Move `ExportEntry::module_request` field to last
(#16403) (overlookmotel)

### ⚡ Performance

- 790beeb napi/parser: Do not remove extraneous options on JS side
(#16447) (overlookmotel)

Co-authored-by: Boshen <[email protected]>
Copilot AI pushed a commit that referenced this pull request Dec 10, 2025
### 💥 BREAKING CHANGES

- 083fea9 napi/parser: [**BREAKING**] Represent empty optional fields on
JS side as `null` (#16411) (overlookmotel)

### 🚀 Features

- 7a2afee parser: Add TS1174 error for classes extending multiple base
classes (#15993) (sapphi-red)
- da87812 semantic: Add TS2309 error for export assignment with other
exports (#15992) (sapphi-red)
- d6d2bcd minifier: Remove unused function calls that are marked by
`manual_pure_functions` (#16534) (sapphi-red)
- c90f053 minifier: Support `.` separated values for
`compress.treeshake.manualPureFunctions` (#16529) (sapphi-red)
- a607cc4 codegen: Preserve comments between CatchClause's param and
body (#16167) (copilot-swe-agent)
- 8c10694 semantic: Expose get_comment_at method (#16439) (camc314)
- 3981e7a ast: Add get_comment_at to lookup a comment by span (#16438)
(camc314)

### 🐛 Bug Fixes

- 699406a napi/parser: Move `ExportEntry::module_request` field to first
(#16412) (overlookmotel)
- 12bd794 napi/parser: Move `ExportEntry::module_request` field to last
(#16403) (overlookmotel)

### ⚡ Performance

- 790beeb napi/parser: Do not remove extraneous options on JS side
(#16447) (overlookmotel)

Co-authored-by: Boshen <[email protected]>
taearls pushed a commit to taearls/oxc that referenced this pull request Dec 11, 2025
…roject#16447)

Similar to oxc-project#16442.

There's no need to remove the `experimental*` properties from `options` object on JS side. They'll be ignored by NAPI-RS on Rust side anyway, so creating a new object on JS side is pointless overhead.
taearls pushed a commit to taearls/oxc that referenced this pull request Dec 11, 2025
### 💥 BREAKING CHANGES

- 083fea9 napi/parser: [**BREAKING**] Represent empty optional fields on
JS side as `null` (oxc-project#16411) (overlookmotel)

### 🚀 Features

- 7a2afee parser: Add TS1174 error for classes extending multiple base
classes (oxc-project#15993) (sapphi-red)
- da87812 semantic: Add TS2309 error for export assignment with other
exports (oxc-project#15992) (sapphi-red)
- d6d2bcd minifier: Remove unused function calls that are marked by
`manual_pure_functions` (oxc-project#16534) (sapphi-red)
- c90f053 minifier: Support `.` separated values for
`compress.treeshake.manualPureFunctions` (oxc-project#16529) (sapphi-red)
- a607cc4 codegen: Preserve comments between CatchClause's param and
body (oxc-project#16167) (copilot-swe-agent)
- 8c10694 semantic: Expose get_comment_at method (oxc-project#16439) (camc314)
- 3981e7a ast: Add get_comment_at to lookup a comment by span (oxc-project#16438)
(camc314)

### 🐛 Bug Fixes

- 699406a napi/parser: Move `ExportEntry::module_request` field to first
(oxc-project#16412) (overlookmotel)
- 12bd794 napi/parser: Move `ExportEntry::module_request` field to last
(oxc-project#16403) (overlookmotel)

### ⚡ Performance

- 790beeb napi/parser: Do not remove extraneous options on JS side
(oxc-project#16447) (overlookmotel)

Co-authored-by: Boshen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-parser Area - Parser C-performance Category - Solution not expected to change functional behavior, only performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant