-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(es/compat): Apply Array.prototype.slice
to arguments
in loose spread
#11122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 7ca6fd2 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
CodSpeed Performance ReportMerging #11122 will not alter performanceComparing Summary
|
There was a problem hiding this 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 fixes the handling of the arguments
object in loose spread transformations for ES2015 compatibility. The fix ensures that when arguments
is used in a spread expression, it is properly converted using Array.prototype.slice.call(arguments)
to create a real array before spreading.
- Adds special handling for the
arguments
identifier in spread expressions - Transforms
...arguments
to useArray.prototype.slice.call(arguments)
for proper array conversion
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
crates/swc_ecma_compat_es2015/src/spread.rs | Implements special case handling for arguments in spread expressions by wrapping it with Array.prototype.slice.call() |
crates/swc/tests/fixture/issues-10xxx/11118/input/index.js | Test input file demonstrating spread usage with arguments |
crates/swc/tests/fixture/issues-10xxx/11118/input/.swcrc | Test configuration with loose mode enabled |
crates/swc/tests/fixture/issues-10xxx/11118/output/index.js | Expected output showing proper transformation to Array.prototype.slice.call(arguments) |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
let expr = match *expr { | ||
Expr::Ident(Ident { ref sym, .. }) if &**sym == "arguments" => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider extracting the string literal 'arguments' into a constant to avoid magic strings and improve maintainability.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@@ -0,0 +1,7 @@ | |||
function xxx() { | |||
var b = [].concat(Array.prototype.slice.call(arguments)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Babel optimizes away the [].concat
here. We can iterate on a refinement later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Description:
BREAKING CHANGE:
Related issue (if exists):