denoland
[Decorators] SyntaxError when decorating classes with overloaded constructors
Background Information
Environment
- Deno Version: v1.40.4+d2477f7
- Operating System: macOS Sonoma 14.2
- Other relevant details: Legacy Decorators are disabled
Bug Description
When a class with overloaded constructors and decorated members is used, Deno v1.40.4+d2477f7 throws a SyntaxError indicating that a class may only have one constructor. This error does not occur in classes without decorators, implying that the combination of decorators and overloaded constructors triggers the syntax error.
Expected Behavior
Decorators should not interfere with the definition of a class with overloaded constructors. Such classes should compile without syntax errors, adhering to valid TypeScript syntax.
Steps to Reproduce
- Define a class with at least one constructor overload signature.
- Apply a decorator to any class member or the class itself.
- Import the class into the Deno REPL or a TypeScript/JavaScript file executed by Deno.
- Encounter the syntax error when attempting to use the class.
Minimal Reproduction
Control Samples
Overloaded Class without Decorators
This control sample, which lacks decorators, does not exhibit any errors, confirming that constructor overloads are not the issue:
export class Color {
constructor(hex: string);
constructor(r: number, g: number, b: number, a?: number);
constructor(r: string | number, g?: number, b?: number, a = 1) {}
get rgba() {
return [0, 0, 0, 1];
}
}
Results:
import { Color } from "./color.ts"
const c = new Color();
console.log(c.rgba); // Outputs: [0, 0, 0, 1]
✅ No errors observed.
Class with Decorators (No Overloads)
Removing overloads while keeping the decorator also works as expected:
const decorate = (target: unknown, context: DecoratorContext) => {
console.log("decorated");
};
export class Color {
constructor(r: string | number, g?: number, b?: number, a = 1) {}
@decorate get rgba() {
return [0, 0, 0, 1];
}
}
Results:
import { Color } from "./color.ts"
// Console logs "decorated"
const c = new Color();
console.log(c.rgba); // Outputs: [0, 0, 0, 1]
✅ No errors observed.
Test Sample
Overloaded Class with Decorators
Adding a decorator to a class member in an overloaded class results in a syntax error:
const decorate = (target: unknown, context: DecoratorContext) => {
console.log("decorated");
};
export class Color {
constructor(hex: string);
constructor(r: number, g: number, b: number, a?: number);
constructor(r: string | number, g?: number, b?: number, a = 1) {}
@decorate get rgba() {
return [0, 0, 0, 1];
}
}
Error Message:
Uncaught SyntaxError: A class may only have one constructor at file:///path/to/color.ts:13:3
❌ This is unexpected behavior.
Final Thoughts
As a die-hard fan of Deno, I can't lie here — I'm confused and concerned that the Deno team decided this decorator implementation was up-to snuff to roll out in a major release. It seems like it was added on a very short time schedule and wasn't adequately tested before the release.
I'm not trying to tell anyone how to do their job, and I'm certainly not trying to be insulting, but this bug (as well as #22253 and #22254), really should've been caught by unit tests rather than an end user such as myself. It's hardly an edge case scenario. At the very least, I would've expected this to have been released behind a flag like --unstable-decorators until it was actually stable.
Regarding SWC
After diving into the SWC code that Deno relies on for decorator transformations, it's painfully clear that SWC is the cause of not just this syntax error, but virtually all the other decorator-related headaches we're seeing. I get that SWC is its own project, and I'm not here to debate their standards or release strategy. However, this situation begs the question: Are there no alternatives to SWC that stick closer to the decorator spec, or at least, don't break every project using decorators in Deno?
Possible Solutions and Alternatives?
I'm not suggesting to get rid of SWC altogether. I understand that Deno relies on SWC for much more than Decorators alone, and it would be ludicrous for me to suggest you switch compilers over something like this. I'm actually quite a fan of SWC's, but I'm also a big nerd when it comes to spec-compliance. Could there be a solution that allows us to have the best of both worlds, continuing to benefit from the speed of SWC's while also enjoying spec-compliant decorators?
Forgive me and my naïvité in regards to how Deno, TypeScript, and SWC work together to transform the decorators; I'm not familiar with the intricacies of that section of the project, so at the risk of sounding foolish... would it be possible to leverage Deno's bundled version of the TypeScript compiler and use its built-in __esDecorator helper instead of SWC's?
If you compare the code between the two helpers side-by-side, it's very obvious which one is superior. The code for SWC's helper is located in helpers/_apply_decs_2203_r.js. I couldn't find a direct link to the __esDecorate helper in the TypeScript repo, so I've pasted it below from a class that I just compiled down to ES2022.
TypeScript's emitted `__esDecorate` helper
var __esDecorate = (this && this.__esDecorate) || function (ctor, descriptorIn, decorators, contextIn, initializers, extraInitializers) {
function accept(f) { if (f !== void 0 && typeof f !== "function") throw new TypeError("Function expected"); return f; }
var kind = contextIn.kind, key = kind === "getter" ? "get" : kind === "setter" ? "set" : "value";
var target = !descriptorIn && ctor ? contextIn["static"] ? ctor : ctor.prototype : null;
var descriptor = descriptorIn || (target ? Object.getOwnPropertyDescriptor(target, contextIn.name) : {});
var _, done = false;
for (var i = decorators.length - 1; i >= 0; i--) {
var context = {};
for (var p in contextIn) context[p] = p === "access" ? {} : contextIn[p];
for (var p in contextIn.access) context.access[p] = contextIn.access[p];
context.addInitializer = function (f) { if (done) throw new TypeError("Cannot add initializers after decoration has completed"); extraInitializers.push(accept(f || null)); };
var result = (0, decorators[i])(kind === "accessor" ? { get: descriptor.get, set: descriptor.set } : descriptor[key], context);
if (kind === "accessor") {
if (result === void 0) continue;
if (result === null || typeof result !== "object") throw new TypeError("Object expected");
if (_ = accept(result.get)) descriptor.get = _;
if (_ = accept(result.set)) descriptor.set = _;
if (_ = accept(result.init)) initializers.unshift(_);
}
else if (_ = accept(result)) {
if (kind === "field") initializers.unshift(_);
else descriptor[key] = _;
}
}
if (target) Object.defineProperty(target, contextIn.name, descriptor);
done = true;
};
var __runInitializers = (this && this.__runInitializers) || function (thisArg, initializers, value) {
var useValue = arguments.length > 2;
for (var i = 0; i < initializers.length; i++) {
value = useValue ? initializers[i].call(thisArg, value) : initializers[i].call(thisArg);
}
return useValue ? value : void 0;
};
Anyways, if you made it this far, thanks for taking the time to read all of this! I'm very passionate about Deno, and I'm eager to see its continued success and adoption in the JavaScript ecosystem. If there's anything I can do to help solve this issue or implement a fix, please don't hesitate to let me know. I'm open to any discussion and comments you may have on the topic.
Thank you for your time.
— Nick
