Allow the Value type to specify what it can be set to

[funwolf7]

NOTE: everything here assumes that #259 will be fixed. For testing purposes, I have modified the StateObject type to the following, but everything here should work regardless of the fix used.

export type StateObject<T> = Dependency & {
	type: "State",
	kind: string,
	_typeIdentifier: (never) -> T
}

---

The Problem

(all code samples shown best work in strict mode)

Currently in Fusion, the Value type can be converted so that its generic can be made looser. This means that the following code is allowed:

local foo: Fusion.Value<string> = Value("foobar")
local bar: Fusion.Value<string | number> = foo

While this may seem fine at first glance, it can be a problem, specifically for functions that make use of :set(). Consider the following contrived example:

local function modifyValue(val: Fusion.Value<number | string>): ()
	local currentValue: number | string = peek(val)
	val:set(tostring(currentValue) .. "-" .. type(currentValue))
end

local bar: Fusion.Value<number | string> = Value(1)
modifyValue(bar) -- This is completely fine and will never cause problems

local baz: Fusion.Value<string> = Value("baz")
modifyValue(baz) -- This is completely fine and will never cause problems

local foo: Fusion.Value<number> = Value(5)
modifyValue(foo) -- This should not be allowed

print(peek(foo) + 5) -- Errors, but doesn't give a type error

Even if Value:set() was correctly typechecked (it isn't currently), this would still be allowed by the current system. The problem is that both ends see it as okay, but as a whole it isn't.

From the perspective of the code that is calling the function, foo can contain number, which can be made looser into number | string, so passing it into the function is allowed. From the perspective of the function, val can contain either a number or a string, so setting it to a string should be allowed. However, looking at the whole code, it is obvious that it will error, as foo gets set to be a string, and the code that handles foo doesn't expect that.

The only "fix" for this that uses only one generic type would be to make it so that the Value type cannot be changed whatsoever, neither by loosening it nor by strictening it. However this limits what code can do. Consider the baz variable. Passing it to modifyValue would never cause any errors, but with the proposed immutability of the Value type, it wouldn't be allowed. This also causes problems with the creation of Value objects, as the inferred type will likely be stricter than the actual type you want (this is my guess as to why Value:set() isn't currently typechecked).

Potential Solution

To solve these issues, a second generic type could be used to separate what a Value may contain and what a Value can be set to. For demonstration purposes, I will change the Value<T> type to Value<T, S>, where T represents what peek or use can return, and S represents what :set() can be called with. Value<T, T> would be similar to the current Value type.

The T type would behave like it currently does, and can be made looser but not stricter. This means that number can become number | string, but not vice versa. The S type would behave the opposite; it can be made stricter but not looser. This means that number | string can become number, but not vice versa.

A lot of the time (especially for variables), T and S will be the same, so to simplify this, the S generic should default to T.

The reasoning for why these types behave this way can be seen by examples. Let's try to fix the previous problematic code. We want to say that modifyValue can handle val containing either a number or string, but also warn that it will set val to contain a string, so any code relying on val must be able to handle that.

local function modifyValue(val: Fusion.Value<number | string, string>): ()
	local currentValue: number | string = peek(val)
	val:set(tostring(currentValue) .. "-" .. type(currentValue))
end

local bar: Fusion.Value<number | string> = Value(1)
modifyValue(bar) -- No type errors

local baz: Fusion.Value<string> = Value("baz")
modifyValue(baz) -- No type errors

local foo: Fusion.Value<number> = Value(5)
modifyValue(foo) -- Type error

From the bar variable, we can see that S=number | string can be converted to S=string without issue. This makes sense, as if val is allowed to be set to either a number or string, then only setting it to a string is fine.

From the baz variable, we can see that T=string can be converted to T=number | string without issue. This makes sense, as if the function can handle val containing either a number or string, it can handle val only containing a string.

Implementation

The current Value type looks like so:

export type Value<T> = StateObject<T> & {
	kind: "State",
 	set: (Value<T>, newValue: any, force: boolean?) -> ()
}

Since the proposed new behavior of T is the same as the current T, we don't need to modify the generic. For implementing the S generic, we should specify the default as T, and change the newValue: any to newValue: S. Since S would then be the parameter of a function, it can only be made stricter, not looser. The final code would be so:

export type Value<T, S=T> = StateObject<T> & {
	kind: "State",
 	set: (Value<T, S>, newValue: S, force: boolean?) -> ()
}

A small flaw with this is that, while S is assumed to be a subset of T, nothing is stopping you from not making it one. A potential solution would be to change the StateObject<T> to StateObject<T | S>. This would change what T represents (peek and use could return more than just T), but would make sure that code can handle anything that the Value can be set to, and even works fine when S is any. If we want to do so, the version of the code would be so:

export type Value<T, S=T> = StateObject<T | S> & {
	kind: "State",
 	set: (Value<T, S>, newValue: S, force: boolean?) -> ()
}

In the init.lua for the library, the type should be changed as well:

export type Value<T, S=T> = PubTypes.Value<T, S>

The Value constructor must be fixed. Since it creates a new Value, no code will rely on it only being set to the inferred type, so it is safe to make S be any (this is necessary, as the inferred type T will likely be stricter than what you want), like so:

Value: <T>(initialValue: T) -> Value<T, any>,

[dphfox]

I'm really grateful for the detail here since I'm not really well versed in Luau type syntax. This definitely was an oversight.