Skip to content
This repository has been archived by the owner on Oct 4, 2023. It is now read-only.

Fix cache types #2699

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix cache types #2699

wants to merge 1 commit into from

Conversation

raymondjacobson
Copy link
Member

@raymondjacobson raymondjacobson commented Jan 27, 2023

Description

These types are hard, but this fixes them partially.
Getting cache entries in bulk were lying that they were Record<string, User|Track|Collection>, but it's really a partial since for whatever reason the value may not actually end up in the cache.

This deserves a bit better of a cleanup, but hopefully this can prevent some errors like missing user_ids on undefined in downstream components, etc.

I think we may consider filtering undefined's out when coming back from the cache too, but that also probably leads to some undefined behaviors and I felt that doing them in the downstream selectors / components was safer.

Dragons

Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.

Ran all type checks & ran app locally web & mobile

How will this change be monitored?

For features that are critical or could fail silently please describe the monitoring/alerting being added.

Feature Flags

Are all new features properly feature flagged? Describe added feature flags.

Copy link
Contributor

@dylanjeffers dylanjeffers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow amazing changes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants