Support RBXScriptSignal-Compatible Events

[TheNexusAvenger]

I am trying to use Fusion with a custom button implementation (Nexus Button) that tries to act like a normal instance in terms of property and event passthrough. A quirk of it is that events are custom, but are API-compatible with Roblox events. They exist to provide better typing and to prevent the passed values from being encoded + decoded. With some custom supporting code, the custom buttons can be made to work with Fusion, except for the events.

local NexusButton = ...
local Fusion = ...
local applyInstanceProps = ... --Fusion.Instances.applyInstanceProps

local OnEvent = Fusion.OnEvent

local scope = Fusion.scoped(Fusion, {
    Button = function(scope)
        local button = NexusButton.new()
        return function(properties: Fusion.PropertyTable): TextButton
            table.insert(scope, button) --Scopes support custom :Destroy() methods, which NexusButton has.
            applyInstanceProps(scope, properties, button) --applyInstanceProps works with non-instances. NexusButton acts like one despite not being one.
            return button:GetWrappedInstance() --Returns a TextButton.
        end
    end,
})

local myButton = scope:Button()({
    Name = "MyButton",
    BackgroundColor3 = Color3.fromRGB(255, 255, 255)
    [OnEvent("MouseButton1Down")] = function() --[ERROR] Causes "The TextButton class doesn't have an event called 'MouseButton1Down'." due to not being an RBXScriptSignal.
        print("Clicked!")
    end,
})

Having a non-instance that acts like an instance may or may not be cursed, but it is what I am rolling with. Most of Fusion actually works with it except for 2 aspects:

• OnEvent does not allow for API-compatible events at all and throws errors.
• doCleanup doesn't support Disconnect. Destroy is supported, but since RBXScriptConnection does not have that, there isn't an obvious reason to add it.
  • This is fairly easy to work around by mapping Disconnect to Destroy. This is something I can do with a new release, but this might not be a luxury for other people using cursed instance imitation systems.

Both can and have been worked around already (custom OnEvent implementation + store a function to call Disconnect to scope), but I'd be interested in removing the workarounds from my code if I could. I'm also fine with working on a pull request to resolve this if this is something worth addressing.

[dphfox]

Creating imitations of Roblox types does feel a little bit cursed, and I'm not sure there's a strong motive to support them.

I feel like the solution here isn't to relax Fusion's type safety checks, but to build your own special key that implements the type safety checks that are relevant for your project. Part of the reason Fusion's built modularly is precisely so that these primitives can be replaced when Fusion's generic opinions don't gel with specific projects - I don't think it'd be worth the compromise on Fusion's end to adopt this as a generic opinion for all consumers.

That said, I'm open to changing my mind on this, but I'm pretty cognisant that this sort of change tends to be the sort of thing introducing forward compatibility problems down the line, especially since it is impossible to perfectly emulate the Instance API and all of its related contracts.

[TheNexusAvenger]

I feel like the solution here isn't to relax Fusion's type safety checks, but to build your own special key that implements the type safety checks that are relevant for your project.

This is exactly what I have done. It is basically a copy of OnEvent but without the type checks. The only change we would remove is the comment --TODO: Can API-compatible event handling be upstreamed?, by either upstreaming the change or just removing the comment because it won't be accepted as a change. It is just a bit annoying to use because it is super easy to accidentally use OnEvent instead of the custom one and get an error while testing.

That said, I'm open to changing my mind on this, but I'm pretty cognisant that this sort of change tends to be the sort of thing introducing forward compatibility problems down the line, especially since it is impossible to perfectly emulate the Instance API and all of its related contracts.

Acting like Roblox Instances (I think) is a fairly rare use case, because it has some nasty side effects (have fun accidentally mixing them with normal instances outside of your unit tests). The only reason OnEvent and doCleanup supporting API-compatible events is a question is because applyInstanceProps happens to work, but that can't be guaranteed in the future.

From my own tests, the non-test, non-documentation code to support this is about 6 lines of changes. It could be troublesome into the future, or it could not be. It would be a fairly easy pull request for me to make to be judged, or this could simply not be upstreamed and we continue to do our own [cursed] thing. That and I also add Destroy to my custom event connections so I don't have to wrap them in a function before adding to the scope.