Support for function and class keys in New function.

[Gargafield]

Right now there's a lot of requests to add different things to the New function (#36 #19 #1). So I propose the support for functions as keys in the New function. This allows for more modular support.

Here's how it could work:

-- Define our function
-- inst is the instance that New creates
-- props are the value of that key.
function Tag(inst, props)
    if type(props) == "table" then
        for _, tag in ipairs(props) do
            CollectionService:AddTag(inst, tag)
        end
    else
        CollectionService:AddTag(inst, props)
    end
end

-- Using it
local part = New("Part")({
    Name = "Part",
    Parent = workspace,
    Position = Vector3.new(0,0,0),
    [Tag] = {"Part", "Idk"}
})

[dphfox]

This is an interesting idea! I've been looking for ways to make the New function more modular and extensible - in particular, by end users - and this has potential to be the ticket to simple, clean, extensible and understandable code.

I suspect if we did this, it would almost completely replace symbol keys - there's still some questions about how we'd deal with order of operations though (e.g. events have to be connected last)

Ill have to investigate this further 👍🏻

[Gargafield]

This is an interesting idea! I've been looking for ways to make the New function more modular and extensible - in particular, by end users - and this has potential to be the ticket to simple, clean, extensible and understandable code.

I suspect if we did this, it would almost completely replace symbol keys - there's still some questions about how we'd deal with order of operations though (e.g. events have to be connected last)

Ill have to investigate this further 👍🏻

The function needs to run last since they need the instance. There's also the possibility that classes could have a "RunPriority" key.

[boatbomber]

I had an idea on how to do this with extreme simplicity, although it won't cover all cases. It works for my uses though (and the example Tag from OP), so I'll be slapping this into my fork until the official solution is implemented. Figured it's worth sharing.

	--[[
		STEP 2.3: Function keys
	]]
	elseif typeof(key) == "function" then
		-- Defer so instance setup completes before the function runs
		task.defer(key, ref.instance, value)

Edit: A few things of note.

• This solution works well for constants, but not state.
  This defer trick, although a nice one line solution, doesn't play nicely with dependency management afaik.
  Also, we may want to pass the state's value to the user funciton, rather than the state itself, for the sake of making the user function's easer to create and maintain. Something like this:
  task.defer(key, ref.instance, (typeof(value) == "table" and value.type == "State") and value:get() or value)
• This makes it run after events are connected, which may not be desirable in many cases.

[dphfox]

I think it could be interesting to wrap the keys in a table to hold extra metadata, for example execution order. Perhaps it would end up looking something like this if you were to make your own OnEvent implementation:

local function OnEvent(eventName)
    return {
        order = "end",
        callback = function(value, props, instance, tasks)
            table.insert(tasks, instance[eventName]:Connect(value))
        end
    }
end

Or perhaps even something that applies some font settings:

local ApplyFont = {
    order = "start",
    callback = function(value, props, instance, tasks)
        props.TextScaled = true
        props.Font = "Gotham"
        props.TextSize = 16
    end
}

In general I'm on board with the idea of separating out the functionality of OnEvent, OnChange, Children, Ref etc from the New function and making it more modular (especially if this allows for third party additions!) but I'm unsure exactly what these functions should be given access to or how exactly they should work.

[astrealRBLX]

I don't believe exposing the API this much is the correct implementation of this feature. Fusion becoming modular is important but I believe that this can quickly get confusing. I much prefer the idea of having a dedicated API token for use in the New function that will expose Fusion's API. For example:

local function ApplyFont(value, props, instance, tasks) -- Value() objects are not automatically unwrapped
    ...
end

local function OnEvent(eventName)
    return function(value, props, instance, tasks)
        ...
    end
end

New 'TextLabel' {
    ...
    
    [API] = {
        [ApplyFont] = Enum.Font.Gotham,
        [OnEvent 'Activated'] = ...
    }
}

The API token expects its value to be an array where the indices define callback order. For safer and simpler operations I believe a Preprocess and Postprocess token should exist similar to the API token. The difference being what they actually have access to. I don't really like splitting this functionality up into two different tokens but I can't think of a nicer way of going about it.

local function Tag(inst, props)
    if type(props) == "table" then
        for _, tag in ipairs(props) do
            CollectionService:AddTag(inst, tag)
        end
    else
        CollectionService:AddTag(inst, props)
    end
end

New 'TextLabel' {
    ...
    
    [Preprocess] = { -- Applied after step 1 but before step 2
        [Tag] = {'Hello', 'World'} -- If a Value() is used it should automatically be unwrapped when passed to the preprocessor
    },
    
    [Postprocess] = { ... } -- Works the same except applied after step 6
}

[dphfox]

I don't believe exposing the API this much is the correct implementation of this feature. Fusion becoming modular is important but I believe that this can quickly get confusing.

What specifically would you find confusing about the proposed implementation? If we can establish what's wrong with it more definitively, we can work to solve those issues in particular :)

New 'TextLabel' {
    ...
    
    [API] = {
        [ApplyFont] = Enum.Font.Gotham,
        [OnEvent 'Activated'] = ...
    }
}

I really don't like this idea - if we were to go ahead with this, then every single use of a non-string key would have to be wrapped in [API]. This is a massive breaking change. If the purpose is to distinguish what is a regular property assignment and what is special behaviour, the use of a non-string key already indicates this perfectly well. It's nice that users can specify their own order of operations here, but in the vast majority of cases users don't care and want the library to handle this for them, for example with events.

[astrealRBLX]

Alright your argument makes sense + I think code could get cluttered very quickly. When I said things can get confusing what I specifically meant is I don't believe exposing such a large portion of the Fusion API for something as simple as applying tags makes sense. It's worth looking into a proper way to provide a more limited version of the API for simpler and safer tasks like that.

In terms of the actual syntax I had proposed before I think this might work better.

New 'TextLabel' {
    [API(Tag)] = {'Hello', 'World'},
    [API(ApplyFont)] = Enum.Font.Gotham,
    [API(OnEvent 'Activated')] = ...
}

Then question then becomes what should be returned by the function fed into the API() function.

[dphfox]

When I said things can get confusing what I specifically meant is I don't believe exposing such a large portion of the Fusion API for something as simple as applying tags makes sense.

This is a fair point - I'm mainly thinking about third party libraries with these changes, but the tags are simply something easy that people could add in. I'm currently looking to make Fusion much easier to extend to fit a wider range of coding styles and provide custom features that feel as good as first-party features.

New 'TextLabel' {
    [API(Tag)] = {'Hello', 'World'},
    [API(ApplyFont)] = Enum.Font.Gotham,
    [API(OnEvent 'Activated')] = ...
}

What is the purpose of passing everything through an API call here?

[astrealRBLX]

Currently Fusion uses symbols for "special" keys and I believe exposing the API to certain functions should be no different. When wrapped in the API call the passed function will be called with whatever API parameters are exposed. If a raw function is used as a key then it should behave like what the initial issue proposed where it receives only the instance and props. I should've been more clear as to what I was trying to get at hopefully that makes more sense.

[dphfox]

I still don't really understand what you're trying to do here :/

[Gargafield]

I like @astrealRBLX approach. But it's still a bit cluttered.
Here's my idea:
You can set functions as keys and they will be called immediately after creating the instance. The value of the key and the instance is passed to the function.
Using the API function you can add special metadata. Like when the function should be called. Or if it wants any other data, like setting internal values in New.

[Gargafield]

There's a lot of feature request that could be fixed by this issue. It would be better to get 5 components from GitHub than 5 extra functions in fusion.

[dphfox]

Right now I'm going forward with this design for the New rewrite:

local Children = {
    type = "SpecialKey", -- identifies this as a special/function key
    kind = "Children", -- for type checking/debugging
    step = "descendants", -- when should this key be run during the property application process?
    apply = function() ... end -- applies this to the instance
}

I'm willing to revisit this in the future, but right now this is the design I have the most confidence in. Plus, we have to settle on something for now.

[dphfox]

I think we've settled on the current design for now.