Skip to content

Commit

Permalink
Remove DEFAULT_STRING_ID and add more instructions
Browse files Browse the repository at this point in the history
  • Loading branch information
fabioh8010 committed Nov 12, 2024
1 parent d1e7405 commit 7bcabe0
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 7 deletions.
8 changes: 5 additions & 3 deletions contributingGuides/STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -477,13 +477,15 @@ if (ref.current && 'getBoundingClientRect' in ref.current) {

### Default value for inexistent IDs

Use `CONST.DEFAULT_NUMBER_ID` when there is a possibility that the number ID property of an Onyx value could be `null` or `undefined`. **Do not default string IDs to any value unless absolutely necessary**, in case it's necessary use `CONST.DEFAULT_STRING_ID` instead.
Use `CONST.DEFAULT_NUMBER_ID` when there is a possibility that the number ID property of an Onyx value could be `null` or `undefined`. **Do not default string IDs to any value!**

> Why? The default number ID (currently set to `0`, which matches the backend’s default) is a falsy value. This makes it compatible with conditions that check if an ID is set, such as `if (!ownerAccountID) {`. Since it’s stored as a constant, it can easily be changed across the codebase if needed.
>
>
> However, defaulting string IDs to `'0'` breaks such conditions because `'0'` is a truthy value in JavaScript. Defaulting to `''` avoids this issue, but it can cause crashes or bugs if the ID is passed to Onyx. This is because `''` could accidentally subscribe to an entire Onyx collection instead of a single record.
>
>
> To address both problems, string IDs **should not have a default value**. This approach allows conditions like `if (!policyID) {` to work correctly, as `undefined` is a falsy value. At the same time, it prevents Onyx bugs: if `policyID` is used to subscribe to a specific Onyx record, a `policy_undefined` key will be used, and Onyx won’t return any records.
>
> In case you are confused or find a situation where you can't apply the rules mentioned above, please raise your question in the [`#expensify-open-source`](https://expensify.slack.com/archives/C01GTK53T8Q) Slack channel.
``` ts
// BAD
Expand Down
4 changes: 0 additions & 4 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ const EMPTY_OBJECT = Object.freeze({});

const DEFAULT_NUMBER_ID = 0;

/** Only default a string ID to this value if absolutely necessary! */
const DEFAULT_STRING_ID = '';

const CLOUDFRONT_DOMAIN = 'cloudfront.net';
const CLOUDFRONT_URL = `https://d2k5nsl2zxldvw.${CLOUDFRONT_DOMAIN}`;
const ACTIVE_EXPENSIFY_URL = Url.addTrailingForwardSlash(Config?.NEW_EXPENSIFY_URL ?? 'https://new.expensify.com');
Expand Down Expand Up @@ -838,7 +835,6 @@ const CONST = {
EMPTY_ARRAY,
EMPTY_OBJECT,
DEFAULT_NUMBER_ID,
DEFAULT_STRING_ID,
USE_EXPENSIFY_URL,
EXPENSIFY_URL,
GOOGLE_MEET_URL_ANDROID: 'https://meet.google.com',
Expand Down

0 comments on commit 7bcabe0

Please sign in to comment.