Skip to content

Centralize how time values are exposed throughout the frontend #19342

@Parkreiner

Description

@Parkreiner

This is an issue to encapsulate the remaining work needed to bring #17587 back from the dead.

Problem

A lot of our React logic is heavily reliant on time values. In many cases, we can get away with using dates and timestamps generated from the API (which also makes it easier to mock that data out), but in several places, we need to have client-generated time/Date values, and it's not always clear how they should be handled.

Since this is React, there are basically two "zones" where time values can come into the picture:

  • The main body of each component, which defines the render logic. Per React rules, all render should be 100% pure and deterministic
  • Effects and actions (i.e., useEffect callbacks, event handlers) – These are allowed to be as imperative, messy, and mutative as you want. We can basically do whatever time-based logic we want here, because the logic ostensibly "lives outside React"

There shouldn't be too many concerns with the effects/actions. What is a concern is the render logic. We have a lot of new Date() calls inside the render paths, which has been causing a lot of problems for us:

  • Because the date values usually aren't tied to any specific state, we're stuck in a position where we just have to pray that our functions render often enough to avoid stale timestamps. For a lot of the codebase, we have zero control over how a timestamp actually makes its way into what's shown on screen
  • Because render output isn't 100% deterministic, that causes test flakes, both in Storybook and our other tests. For Storybook specifically, we've had to rely on adding a bunch of band-aid data attributes to tell Storybook to ignore parts of the output. The side effect of this is that there are parts of the UI that could accidentally break overnight without anyone knowing, because Storybook won't flag it.
  • Another factor is that because each component typically doesn't share the same time value to its children, different components each have their own independent time values. This is usually fine for the initial render, but as different components continue to re-render over time, there's an increased risk that you can see different parts of the UI start to contradict each other (all on the same screen), because some parts will update, while others won't

Proposed solution

I think that the best way to fix all of this would be to create a new custom React hook that lets us centralize most/all our time logic as a single, deterministic, dependency-injectable Date object. (This will be replaced with a Temporal object once that API has enough adoption). This has a few benefits:

  • In production, all components are guaranteed to be based off the same time value. This means that Dates cannot randomly change without the underlying Date state changing
  • We have the ability to inject a specific Date for all our stories, meaning that all our stories have much higher guarantees of being deterministic (some of our components still have other problems, but I'm hoping to take care of those in other PRs).

Additional non-functional ideas

These are additional ideas I have to make the solution feel a bit more polished, but they ultimately aren't required for an initial solution:

  • Make everything subscription-based via useSyncExternalStore – this makes it really easy to let components subscribe and unsubscribe to changes for the central Date object, while avoiding screen flickering and other "UI tearing" issues
  • Make it so that we can "select" derived values from the main Date object. This lets us make our subscriptions "fine-grained" as a performance optimizaiton. That is, if a component only needs the year from the Date state, and the year is the same after the state changes, that should not trigger a re-render

Remaining work to be done

  • Revisit work in the current draft PR and see what still makes sense
  • Finish up all stub methods and parts of the implementation marked with @todo
  • Add tests to validate that functionality is working properly
  • Wire the hook up to all necessary components, and erase all data attributes that told Storybook to ignore time-based parts of the UI

Metadata

Metadata

Assignees

Labels

javascriptPull requests that update javascript codesiteArea: frontend dashboard

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions