-
Notifications
You must be signed in to change notification settings - Fork 30
Description
Problem
FlowMVI provides useful functions to prevent state racing through updateState and withState functions, but sometimes the way it's declared can lead to potential problem overlooking. The problem is that the state is handled in the receiver of the lambda, not as a parameter.
For example:
reduce { intent ->
when (intent) {
is EmailChange -> updateState { copy(email = EmailAddress(intent.value)) }
is ButtonClicked -> updateState {
copy(email = email.validate()).let {
if (email.isValid) { // we used previous state that isn't validated, what means it'll always be false
it.copy(isLoading = true)
authorizeAsync(..)
} else it
}
}
}
}Possible solution
To avoid such problems, but within parameters in the chain of inner lambdas, Intellij Idea has dedicated inspection that warns when you have the same parameters names. For the receivers, there's no such validation or check (except of DSLs contexts) and it makes updateState and withState potentially problematic even in simple cases.
Introducing the separate functions without such problem can help, but does not solve problem fully as we can't be sure that someone else will use the right function. In addition, it adds a new layer of complexity, as there's already useState, withState and updateState that you should understand.
What would I do?
- Deprecate actual function with, at least,
DeprecationLevel.WARNINGlevel or withRequiresOptIn(it will make able people to opt-in the error / warning on the project level and will not break existing code). - Introduce new functions that will provide state as a parameter, but not as receiver.
Overall, I think everything except of DSLs builders that requires particular contexts to run, should be handled using parameters as it's more obvious in case I described and, for example, when looking on code on platforms like GitHub, but not in IDE.