Trailing comma incorrectly kept when stripping types for ...rest param

[danopia]

Describe the bug

When accepting an options argument in a function, often times I want certain options to be kept separate. I achieve this by setting up an object splat, or whatever they're called, and putting a rest param afterwards to catch the rest.

The issue that I see is that swc is lifting out the trailing comma, if any, from within the TypeScript type clause for the overall parameter, and dropping it into the splat which creates a syntax error.

Input code

function myFunc({...data}: {
  field: string,
}) {
}

Output code

// Uncaught SyntaxError: Rest element must be last element
function myFunc({ ...data,  }) {
}

Note that removing the type's inner trailing comma removes it from the output as well, producing valid code:

Input code

function myFunc({...data}: {
  field: string
}) {
}

Output code

// All good!
function myFunc({ ...data  }) {
}

Config

I'm nominally using Deno, so to recreate this bug I wrote a very basic .swcrc:

{
  "jsc": {
    "parser": {
      "syntax": "typescript"
    },
    "target": "es2020"
  }
}

Expected behavior
The inner comma probably shouldn't get lifted out of the type signatures in general. I don't see when that would be useful

Version

repro@1.0.0
├── @swc/cli@0.1.36
└── @swc/core@1.2.52

Additional context
It seems that using semicolons instead of commas inside the type is a valid workaround.

[Liamolucko]

It seems like this is the cause:

swc/ecmascript/codegen/src/lib.rs

Lines 1629 to 1645 in df2a926

	// Write a trailing comma, if requested.
	let has_trailing_comma = format.contains(ListFormat::AllowTrailingComma) && {
	if parent_node.is_dummy() {
	false
	} else {
	match self.cm.span_to_snippet(parent_node) {
	Ok(snippet) => {
	if snippet.len() < 3 {
	false
	} else {
	snippet[..snippet.len() - 1].trim().ends_with(',')
	}
	}
	_ => false,
	}
	}
	};

It tries to emit a trailing comma if the input has a trailing comma, but this picks up the trailing comma of the type annotation. Should this functionality just be removed?

[danopia]

It looks like a fix for this e3b64a0 is rolled into this PR #1572

[kdy1]

@danopia You are correct, thanks! I'll update the pr.

[swc-bot]

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.