Today I worked on an interesting bug in a fairly modern codebase and I thought I’ll jot down some notes.
TL;DR be careful how you implement “parse; don’t validate”
The Problem
This is on the PDP page for a product that contains multiple attributes (like platform, edition, condition etc.). Product availability is determined based on the URL. For business reasons, we chose to implement this as a useEffect call after page load.
The functionality is essentially as follows. Availability info for all combinations is provided as a JSON blob on page load. This was a tradeoff we made so subsequent selections are faster.
If a user lands on a PDP page, we determine their selected combination and query the JSON for availability data. For eg. if a user lands on a page where
- Platform: Playstation 5
- Condition: New
- Edition: Gold
We pass these selections to the JSON and get back which variations of the product are available for the given selections.
The problem, however, was we were noticing a discrepancy between how the UI looked on page load vs a few seconds later. Since a picture is worth a thousand words see below:
For context, when things are loading ALL variations were marked as unavailable. Once the page finished loading, we were seeing the correct version of the page where some combinations were available and some weren’t.
Even though this all happened within a second or so, it was discernible and needed to be fixed.
Before
After
Side by side comparison
The context
This is a fairly modern codebase that is meant to be reactive. We use Next JS, React, Typescript and Zustand for state management. We have a fairly type driven, functional style codebase with modern tools.
All of this to say I don’t expect such a disgusting bug to show up in a codebase like this.
The root cause
I narrowed down the problem to how data was flowing within the application and also how we were parsing the data. The following diagram summarizes how we were implementing the UI.
The diagram seems fairly complicated but the flow is as follows.
Imagine a simple UI component with enabled and disabled states. Imagine a page level Next JS component that received some data from the server, parses it and supplies it to the UI component. Parts of this data are stored in a global store. The root cause of the problem was the delay between when the page load triggered the useEffect
and when the global state was updated with data that needed to flow back to the component. That’s not all but let’s dive into this for now:
- Page loads and requests for global state
- Global state is not yet fully setup so sends the default state (false)
- The page component passes this state to the parser
- The parser sends the
false
to the UI component - Page load finishes and a
useEffect
call is triggered - The correct state is determined and global state is set
- Changing global state triggers a re-render of the page component
- New state
true
is passed to the parser true
is passed down to the UI component which now shows the correct state
The time difference between the default state showing and how long it takes for us to determine the actual value is where the bug lie.
Also, there is a significant time difference because there is a series of nested useMemo
calls underneath.
Parse; Don’t validate
As you can see in the flow above, we don’t simply pass down data to the store. We use a parser, which is very handy to construct the page model.
It helps us achieve the “parse; don’t validate” style of programming. If you don’t know what that is, you can learn more about it in this beautiful article.
Essentially, instead of determining if something is valid and then throwing away that information, parse don’t validate, encourages you to construct the model you need as you are validating it.
In addition, instead of throwing away all the info you have, you parse whatever info you can and return the model instead of a simple isValid
boolean.
This way there isn’t any throwaway work but the tradeoff is you are passing the buck to the caller of the function to determine if what they received is a Result
or a Maybe Result
.
This brings us to the root cause of our problem. Take a look at the function signature below:
const groupAttributes = (
product: ProductResponse,
selectedAttributes: Record<string, string>
): AllAttributes
Without going into the details of implementation, all this function does is take the base data and apply user selections to it and return a data model back that the UI can use to display information.
The problem is, selectedAttributes
is dependent on the useEffect
call. So the first time this function is called, selectedAttributes
evaluates to {}
. Since we use “parse; don’t validate”, this method returns some values as false instead of signifying their non-existence.
We did not implement a version of the model where this data could be in a loading state. Which makes sense when you think about it from a UI perspective. The product is either available or not. So we implemented it as a boolean. But because the implementation had a delay, we now needed a third state - the loading state.
What we really needed was Loading | Result | Error
. Perhaps boolean | null
was an option but that’s brings with it it’s own set of problems.
When loading, it returns false. Once loaded, it returns true. Hence the bug.
Key takeaways
This was a good learning in terms of how you can shoot yourself in the foot if you don’t know what you are doing.
The problem with parse don’t validate is passing the buck. The caller now has to account for the possibility that a piece of data May
not exist.
Using typescript does not automatically guarantee type safety. In other words, problem exists between chair and keyboard.
Account for loading states - this could be a skeleton loader in the UI or a type that was better modeled to indicate a third state that cannot be conveyed by a boolean. is a possibility but null
brings with it its own set of problems.