Class super-constructor is called even when it may not exist

[LastTalon]

In cases where creating types for external Luau libraries, the classes provided are often the Lua "One True Pattern" style classes and these are not necessarily the same as the particular classes emitted by rbxtsc. In particular, the constructor method emitted typically does not exist on these Luau classes, and the library authors would have no reason or thought to include one unless they had some particular idiosyncratic desire to structure it this particular way.

This is normally fine, except when it comes to typing these classes for consumption in roblox-ts code. This would typically look something like in the following example.

declare class A {}

class B extends A {}

https://roblox-ts.com/playground/#code/CYUwxgNghgTiAEkoGdnwILwN4F8BQeSq8AQvCAB4AuIAdsGprnkA

In the general case this presents few issues, but as in the provided example, upon careful inspection of the emit, it becomes apparent that B attempts to call a super-constructor that may not exist. This leads to a spectacular failure during runtime being incredibly unclear as to what the error was. In fact, a user looking at their code would see that they haven't done anything wrong, they don't even have a constructor, and checking the type definitions of their library and the implementation of their library looks correct as well. Only the internal emit detail of expecting this constructor method is actually wrong, and worse, the user has no way to resolve this themselves. They need to change the underlying library and/or types to fix this.

While this particular scenario is uncommon in most existing libraries, nothing prevents users from extending any classes in any libraries they use. The following is perfectly valid code and should not fail through no fault of the user nor library author.

import { Loop } from "@rbxts/matter";

class MyLoop extends Loop<never> {}

https://roblox-ts.com/playground/#code/JYWwDg9gTgLgBAbzgGQhMcC+cBmUIhwBEAAlAEYAeMAzgPQgCGMMAplEQNwBQ3AxgBtGNGnACyAT1To4raqwB2AE1HSwAHgWsAbuwB8iTNyA

Yet this falls victim to the exact problem described. And in fact, this happens even if the author of the type definitions attempts to work around this as well.

interface A {
	new(): A
}

declare const A: A

class B extends A {}

https://roblox-ts.com/playground/#code/JYOwLgpgTgZghgYwgAgILIN4CgCQIIDuAFAJQBcaWAvllgCYQIA2cUKCA9iAM5hoWpazON27IAQsggAPSCDpj0GGkA

As a result, the end effect is that extending Luau classes can't be done unless the Luau library author knows (for some reason) to include a constructor method, that they likely don't even need.

While there hasn't been a lot of discussion on this, one potential solution is to simply change the emit to include the following check.

function B:constructor(...)
-    super.constructor(self, ...)
+    if super.constructor then
+        super.constructor(self, ...)
+    end
end

There has already been a large amount of discussion in the roblox-ts discord server about why this is problematic and why this is needed. Unfortunately it's a bit long and scattered here is a link mid-discussion that summarizes the issue well, and we can continue discussion in this issue. https://discord.com/channels/476080952636997633/494540040991539200/1147320609932578939

[Fireboltofdeath]

The proposed solution here would not work as roblox-ts classes never call .new() on extended classes. The entirety of the work is done in the constructor, including instantiating the super component.

local A = {};
A.__index = A;

function A.new()
	local self = setmetatable({}, A);
	print("Construct A!")
	self.a = 1
	self.b = 2
	self.c = 3
	return self
end

-- Compiled with roblox-ts v2.1.1
local B
do
	local super = A
	B = setmetatable({}, {
		__tostring = function()
			return "B"
		end,
		__index = super,
	})
	B.__index = B
	function B.new(...)
		local self = setmetatable({}, B)
		return self:constructor(...) or self
	end
	function B:constructor(...)
		if super.constructor then
			super.constructor(self, ...)
		end
		print("CONSTRUCT B")
	end
end

local b = B.new();
print(b.a, b.b, b.c);

I would propose a JSDoc tag, or possibly even a type-based tag, (e.g using interface merging), which allows an alternate emit for annotating lua-style classes. This would also introduce the desired behavior in a way that won't cause breaking changes to the existing class implementation, where a larger rework (such as converting roblox-ts classes to be identical to Lua classes) likely would.

I'd expect the alternative emit to look something like

local B
do
	local super = A
	B = setmetatable({}, {
		__tostring = function()
			return "B"
		end,
		__index = super,
	})
	B.__index = B
	function B.new(...)
		local self = setmetatable({}, B)
		return self:constructor(...) or self
	end
	function B:constructor(...)
		return super.new(...)
	end
end

[LastTalon]

That solution sounds good to me. It alleviates most of the pressure on most users and allows an easy way to achieve this for type definition authors. This also makes it easier to PR for users running into this issue.

There are a couple things to consider about this. The first is that whether to call new in the first place requires semantic knowledge, so supporting the ability to not call new at all and/or provide a separate constructor is still needed.

Additionally the issue of what new is named or whether perhaps :constructor exists, but is simply named something different exists. Ideally tags can support this as well to make the entire process seamless.