Page MenuHomePhabricator

[native] Introduce `messageStoreMessagesBlocklistTransform`
ClosedPublic

Authored by atul on May 31 2022, 1:54 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jul 5, 12:28 AM
Unknown Object (File)
Tue, Jul 2, 1:26 AM
Unknown Object (File)
Thu, Jun 27, 10:21 AM
Unknown Object (File)
Thu, Jun 27, 10:21 AM
Unknown Object (File)
Thu, Jun 27, 10:21 AM
Unknown Object (File)
Thu, Jun 27, 10:20 AM
Unknown Object (File)
Thu, Jun 27, 10:13 AM
Unknown Object (File)
Mon, Jun 24, 7:21 AM

Details

Summary

After migration 31, we'll no longer want to persist messageStore.messages via redux-persist. However, we DO want to continue persisting everything in messageStore EXCEPT for messages. The blacklist property in persistConfig allows us to specify top-level keys that shouldn't be persisted. However, we aren't able to specify nested keys in blacklist. As a result, if we want to prevent nested keys from being persisted we'll need to use createTransform(...) to specify an inbound function that allows us to modify the state object before it's passed through JSON.stringify(...) and written to disk. We specify the keys for which this transformation should be executed in the whitelist property of the config object that's passed to createTransform(...).

Test Plan

Included this transform in persistConfig and completed the migration to version: 31 without issue.

Will be more thorough and look at Redux dev tools/REHYDRATE action/etc. before landing this diff.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul requested review of this revision.May 31 2022, 2:01 PM
tomek added inline comments.
native/redux/persist.js
379 ↗(On Diff #13261)

What do you think about using blacklist instead of blocklist?

380–383 ↗(On Diff #13261)

We can make it shorter, but probably flow will complain about it

This revision is now accepted and ready to land.Jun 1 2022, 7:42 AM
native/redux/persist.js
379 ↗(On Diff #13261)

I think generally people are moving away from whitelist/blacklist to allowlist/blocklist... but I don't feel particularly strongly.

It might make sense to keep the vocabulary consistent with redux-persist, but it also seems like based on their GitHub Issues they'll end up moving to allowlist/blocklist (probably not as a breaking change, but as an equivalent) at some point

380–383 ↗(On Diff #13261)

I'll give it a try and update this diff if there aren't any flow issues

native/redux/persist.js
380–383 ↗(On Diff #13261)

No issues with flow, I wonder if it's maybe too clever? Wonder if someone reading the code would grasp what's happening as quickly

rebase before landing

(opted for the more verbose messageStoreMessagesBlocklistTransform and "blocklist" language")

This revision was landed with ongoing or failed builds.Jun 1 2022, 8:59 AM
This revision was automatically updated to reflect the committed changes.
native/redux/persist.js
380–383 ↗(On Diff #13261)

For me destructuring function parameters is a really convenient feature, but sure, if we think it is less readable we can use something more wordy.