[RFC] Refactor FusionAuthProvider
Closed this issue · 5 comments
NOTE: this issue tracks the discussion around the decision to even do this. This is not the implementation issue.
Situation
The FusionAuth React SDK has a FusionAuthProvider component that is responsible for much of the functionality. It's basically the valuable intellectual property of the SDK.
Challenge
The FusionAuthProvider source code is a bit unwieldy and difficult to digest for developers. This is turn can make it difficult to debug or just generally understand what "state" the running code is in. Extending or adding new functionality can also be a precarious operation.
Question
Should we refactor the FusionAuthProvider to improve the readability and maintainability of the code?
Answer
Yes. And let's use XState to model the business logic and state of the FusionAuthProvider.
What is XState?
XState is a state management and orchestration solution for JavaScript and TypeScript apps. It lets us create state machines which offer predictability via guarantees about state and functionality.
Sounds cool but what benefits will this offer us?
Here's an example we can look at.
The example demonstrates a few nifty features:
1. Discrete, exclusive states
In in the world of state machines, the term "state" refers to a "state of being" (as opposed to "data" – this is a common conflation of terms). State machines can only be in one state at a time. In the case of lampMachine, the defined states are TurnedOn and TurnedOff. So the lampMachine can either be TurnedOn or TurnedOff. There's no way to be in both or neither. This is great because it exactly models the binary state lamps actually have.
How is this applicable to the FusionAuth React SDK?
There are two super important states to consider within the FusionAuth ReactSDK: 1) "user is authenticated" and 2) "user is not authenticated". If a user is authenticated they cannot also be unauthenticated (and vice versa). Boolean values can represent this simple idea easy enough. But things get hazy once we introduce related states.
Take this code from the React SDK for example:
const [isLoading, setIsLoading] = useState(false);
const isAuthenticated = useMemo(() => Object.keys(user).length > 0, [user]);So it turns out there's some more "states" we can be in. How do they interact? What's the relationship? Do they affect one another? Does isLoading have something to do with isAuthenticated?
Further, it's possible for isLoading and isAuthenticated to both be true. Is that ok? It's really hard to tell, we need a lot more context which requires digging into the code.
This demonstrates one of the big benefits of state machines: I don't need any more context than reading the defined states to understand what's even possible. Intention is clearly encoded when using state machines.
2. Scoped events
State machines have the ability to scope events to particular states. In the lampMachine, if it's in the TurnedOn state, sending an ON event won't do anything because we didn't define that event for that state. This matches our mental model of how lamps work: if a lamp is turned on, I can't turn it on again. I could try but I couldn't actually do anything since there is nothing to do.
How is this applicable to the FusionAuth React SDK?
When a user is a particular state like Authenticated, we can make sure events that don't contextually mean anything (like AUTHENTICATE) actually do anything at all. We'll be guaranteed to not try and authenticate again.
3. Portable business logic
While there is a small integration API to deal with when consuming state machines, the actual business logic is framework agnostic so can be used in arbitrary JavaScript projects.
How is this applicable to the FusionAuth React SDK?
This means we can build out this robust model of business logic and immediately bring it over to the other SDKs and it'll work exactly the same. This helps maintainability and coherence among the SDKs – a fix for one fixes the others, extending functionality for one, extends that functionality for the others.
What's the lift? 💪
The current business logic isn't very explicit so we will need to be extra careful about interpreting the business logic, extracting it, and translating it into a state machine. We already have built up a good mental model about what's going on but refactors like this have the potential to uncover ambiguity in the current implementation. We'll need to eliminate these ambiguities by making decisions/uncovering what the actual product requirments are. This could involve some back-and-forth or even a meeting.
Estimate: 1 developer 2-3 days
Requesting comments and thoughts from @mooreds.
Requesting comments and thoughts from @lyleschemmerling
I agree that the FusionAuthProvider could be refactored. I am not convinced we need a state machine or a new library. I would rather us not include any additional dependencies unless we absolutely have to. As far as states you are either authenticated or not, and if you are your token is either valid, expiring, or expired. If the state machine is a pattern that solves a problem here then we can do that but should be simple enough to implement on its own.
I believe that we can definitely model the logic in a more robust way with XState but if that isn't something we're necessarily interested in right now I do think we should really consider the portability benefit.
Even if XState doesn't give us more robust state logic, being able to say "here, in this one spot, is all the logic that a FusionAuth Web SDK needs and we can use it in whatever project we want" is a pretty big benefit. XState is framework agnostic so we can use it wherever. It would lead to more cohesiveness among the SDKs if they were able to share the same exact business logic.
Fixing bugs and extending functionality in one spot would be a great developer experience and users would ultimately benefit from that consistency as well since we could iterate faster letting us deliver features and bug fixes faster.
The FusionAuth sdk should ship with as few dependencies as possible and I do not see a need for XState.