[lib] Update `permissionsToBitmask` to `permissionsToBitmaskHex`
ClosedPublic
Actions

Authored by atul on Oct 24 2023, 5:01 PM.

Details

Reviewers

Commits

Summary

Summmary:
Was initially planning (and drafting) to encode the BigInt as base64 to be included in stringified JSON. However, as I jotted down (ENG-5373 : Use "bitmask" approach to encoding ThreadPermissions and Role permissions blobs), decided to encode as hex instead.

Update permissionsToBitmask and corresponding hasPermission to return and accept string instead of bigint.

Test Plan

Unit test, will add a few more.

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

atul published this revision for review.Oct 24 2023, 5:02 PM

tomek added inline comments.

lib/permissions/minimally-encoded-thread-permissions.js
43–48 ↗	(On Diff #32375)	Why do we need this check?

This revision is now accepted and ready to land.Oct 25 2023, 1:09 AM

lib/permissions/minimally-encoded-thread-permissions.js
43–48 ↗	(On Diff #32375)	Flow typechecker doesn't support `BigInt`/`bigint` as of the version we're on, so we do runtime type checking via `invariant` as "the next best thing." Couldn't think of any better approach and didn't want to put this up without any checks

lib/permissions/minimally-encoded-thread-permissions.js
43–48 ↗	(On Diff #32375)	In this particular context, we want to make sure that all bitwise operations are (BigInt, BigInt) or (Number, Number). Mixing the two can lead to undefined behavior (eg losing precision)