chore: Refactor localstorage into typesafe version #17832
Conversation
|
/testenv up |
|
@etr2460 Ephemeral environment spinning up at http://54.201.109.147:8080. Credentials are |
Codecov Report
@@ Coverage Diff @@
## master #17832 +/- ##
==========================================
- Coverage 67.17% 67.16% -0.02%
==========================================
Files 1609 1608 -1
Lines 64797 64798 +1
Branches 6855 6851 -4
==========================================
- Hits 43530 43520 -10
- Misses 19411 19419 +8
- Partials 1856 1859 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -164,7 +164,8 @@ function ChartList(props: ChartListProps) { | |||
| const [preparingExport, setPreparingExport] = useState<boolean>(false); | |||
|
|
|||
| const { userId } = props.user; | |||
| const userKey = getFromLocalStorage(userId?.toString(), null); | |||
| // TODO: Fix usage of localStorage keying on the user id | |||
| const userKey = dangerouslyGetItemDoNotUse(userId?.toString(), null); | |||
There was a problem hiding this comment.
I think userKey may actually be userProfile or userSettings. We can also do type coercing to avoid calling the private dangerous functions:
const userProfile = getItem(String(userId || '') as "FIXME_user_profile_by_user_id", null);
type LocalStorageValues {
// it's a bad practice to use the raw user id as storage keys
// we should at least prefix them.
FIXME_user_profile_by_user_id: {
thumbnails: boolean;
}
}There was a problem hiding this comment.
i'll update the variable name. personally i prefer using the dangerous function here though, since it seems a bit easier to parse than the type assertion (that's clearly not true)
etr2460
force-pushed
the
erik-ritter--localstorage-refactor
branch
from
4caa3b7
to
ce48f9a
Compare
|
Ephemeral environment shutdown and build artifacts deleted. |
mistercrunch
added
🏷️ bot
SUMMARY
Refactors localstorage usage into a typesafe version, and unifies the key constants into an enum so that we can ensure they aren't reused by accident
Precursor to #17708
TESTING INSTRUCTIONS
CI, new unit tests for my changes, test various features in testenv
ADDITIONAL INFORMATION
to: @ktmud @rusackas @graceguo-supercat @pkdotson