Setting self.state to something other than a table only fails when calling setState

[MagiMaster]

For example, if you set self.state to a string, nothing fails immediately. When you later call self:setState, you get a "table expected, got string" error, but attached to the setState line, which makes it hard to trace what went wrong. (Roact should probably raise an error if self.state is not a table after init().)

[ZoteTheMighty]

Digging around a bit, it looks like this is partially related to the logic in assign:

roact/src/assign.lua

Lines 13 to 21 in f55538b

	if source ~= nil then
	for key, value in pairs(source) do
	if value == None then
	target[key] = nil
	else
	target[key] = value
	end
	end
	end

If all of the inputs that we're copying from are empty or nil, then we never even attempt to copy anything to the target and we don't get any errors.

After calling init, we assign the result of getDerivedStateFromProps back onto it:

roact/src/Component.lua

Lines 319 to 324 in f55538b

	instance.state = assign({}, instance:__getDerivedState(instance.props, {}))
	if instance.init ~= nil then
	instance:init(instance.props)
	assign(instance.state, instance:__getDerivedState(instance.props, instance.state))
	end

If getDerivedStateFromProps returns something, then we'll encounter this error sooner. Broadly speaking, users can avoid issues like this by not assigning directly to self.state. But since we need that as a legacy behavior, we should also do a check after we run init that the user left the state as a table or nil.

[MagiMaster]

I thought self.state = {} was still the recommended way to set the initial state in init()?

[ZoteTheMighty]

The best practice as of Roact 1.x is to call setState instead of assigning directly. https://roblox.github.io/roact/api-reference/#init (as well as the examples in the "Guide" section of the docs) should be up-to-date with this.

I suspect this change hasn't been communicated particularly well, though, so we may want to find more ways to promote the new best practice.

All of that said, we should still fix this issue by adding a check in the appropriate place after calling init.

[LPGhatguy]

Is adding a check just after init a sound enough solution?

This condition could happen any time a user reassigns state. We did have the convention of reassigning it in init for a long time, but there's nothing stopping a user from reassigning state (or any other member) to a different value that will later cause issues.

I believe this problem will largely be solved by Luau types once we get them, which will solve this issue in all cases.