fix(isolated-declarations): strip default values from rest parameter binding patterns#17602
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. |
CodSpeed Performance ReportMerging #17602 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the isolated declarations feature where default values in rest parameter binding patterns were incorrectly preserved in generated .d.ts files, causing TypeScript compiler errors. The fix ensures that default values are stripped from rest parameter patterns, consistent with how they are already handled for regular parameters.
Key Changes
- Modified
transform_formal_parametersto recursively remove assignment patterns (default values) from rest parameter binding patterns - Added test cases covering rest parameters with default values in both simple and nested destructuring patterns
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
crates/oxc_isolated_declarations/src/function.rs |
Added logic to strip default values from rest parameter binding patterns using the existing remove_assignments_from_kind visitor |
crates/oxc_isolated_declarations/tests/fixtures/function-parameters.ts |
Added two test cases for rest parameters with default values: simple destructuring and nested destructuring |
crates/oxc_isolated_declarations/tests/snapshots/function-parameters.snap |
Updated snapshot showing correctly stripped default values in generated declarations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Is this a regression caused by #15925? |
Merge activity
|
fed9dbb to
ab5e4ca
Compare
yes |
### 🚀 Features - 659c23e linter: Init note field boilerplate (#17589) (Shrey Sudhir) - 6870b64 parser: Add TS1363 error code (#17609) (Sysix) - 23680a3 mangler: Skip mangling only in scopes affected by direct eval (#17612) (camc314) - a7e1643 parser: Add TS2528 error code to duplicate_default_export diagnostic (#17558) (camc314) ### 🐛 Bug Fixes - 1044116 ecmascript: Mark `new Symbol` as non side-effect free (#17568) (camc314) - ab5e4ca isolated-declarations: Strip default values from rest parameter binding patterns (#17602) (camc314) - 68b2e54 minifier: Prevent incorrect ??= transformation when member base is mutated (#17472) (copilot-swe-agent) ### ⚡ Performance - 6067143 semantic: Remove hash when checking identifier (#17564) (camchenry) - a28ab3d semantic: Avoid bounds check when checking string literal (#17545) (camc314) - 04809d1 semantic: Use SIMD for finding backslashes in `check_string_literal` (#17534) (camchenry) - 49ad2f0 semantic: Mark all diagnostic functions as `#[cold]` (#17487) (camc314) - ea82b50 transformer: Mark all diagnostic functions as `#[cold]` (#17486) (camc314) - d968e51 semantic: Mark `checker::check` as `inline(always)` (#17459) (camc314)

Closes #17598