perf(napi/parser): do not remove extraneous options on JS side#16447
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
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. |
There was a problem hiding this comment.
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.
Merge activity
|
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.
98c53a9 to
35115a7
Compare
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.
35115a7 to
790beeb
Compare
### 💥 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]>
### 💥 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]>
…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.
### 💥 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]>

Similar to #16442.
There's no need to remove the
experimental*properties fromoptionsobject 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.