From ab94ee9076b5708e2deb91e6bff5e9da38cb00de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A1bio=20Henriques?= <contact@fabiohenriques.dev>
Date: Wed, 6 Nov 2024 08:55:25 +0000
Subject: [PATCH] Change guidelines to forbid defaulting values

---
 contributingGuides/STYLE.md | 57 ++++++++++++++++++++++++++++++++-----
 src/CONST.ts                |  7 +++++
 2 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/contributingGuides/STYLE.md b/contributingGuides/STYLE.md
index e6660d848129..755f5228a8a7 100644
--- a/contributingGuides/STYLE.md
+++ b/contributingGuides/STYLE.md
@@ -477,20 +477,63 @@ if (ref.current && 'getBoundingClientRect' in ref.current) {
 
 ### Default value for inexistent IDs
 
- Use `'-1'` or `-1` when there is a possibility that the ID property of an Onyx value could be `null` or `undefined`.
+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.
 
 ``` ts
 // BAD
-const foo = report?.reportID ?? '';
-const bar = report?.reportID ?? '0';
+const accountID = report?.ownerAccountID ?? -1;
+const accountID = report?.ownerAccountID ?? 0;
+const reportID = report?.reportID ?? '-1';
 
-report ? report.reportID : '0';
-report ? report.reportID : '';
+// BAD
+report ? report.ownerAccountID : -1;
+
+// GOOD
+const accountID = report?.ownerAccountID ?? CONST.DEFAULT_NUMBER_ID;
+const accountID = report?.ownerAccountID ?? CONST.DEFAULT_NUMBER_ID;
+const reportID = report?.reportID;
 
 // GOOD
-const foo = report?.reportID ?? '-1';
+report ? report.ownerAccountID : CONST.DEFAULT_NUMBER_ID;
+```
+
+Here are some common cases you may face when fixing your code to remove the default values.
+
+#### **Case 1**: Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
+
+```diff
+-Report.getNewerActions(newestActionCurrentReport?.reportID ?? '-1', newestActionCurrentReport?.reportActionID ?? '-1');
++Report.getNewerActions(newestActionCurrentReport?.reportID, newestActionCurrentReport?.reportActionID);
+```
+
+> error TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string'. Type 'undefined' is not assignable to type 'string'.
+
+We need to change `Report.getNewerActions()` arguments to allow `undefined`. By doing that we could add a condition that return early if one of the parameters are falsy, preventing the code (which is expecting defined IDs) from executing.
+
+```diff
+-function getNewerActions(reportID: string, reportActionID: string) {
++function getNewerActions(reportID: string | undefined, reportActionID: string | undefined) {
++    if (!reportID || !reportActionID) {
++        return;
++    }
+```
+
+#### **Case 2**: Type 'undefined' cannot be used as an index type.
+
+```diff
+function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = false, updatedTransaction, isFromReviewDuplicates = false}: MoneyRequestViewProps) {
+-    const parentReportAction = parentReportActions?.[report?.parentReportActionID ?? '-1'];
++    const parentReportAction = parentReportActions?.[report?.parentReportActionID];
+```
+
+> error TS2538: Type 'undefined' cannot be used as an index type.
+
+This error is inside a component, so we can't just make conditions with early returns here. We can instead use `String(report?.parentReportActionID)` to try to convert the value to `string`. If the value is `undefined` the result string will be `'undefined'`, which will be used to find a record inside `parentReportActions` and, same as `-1`, would find nothing.
 
-report ? report.reportID : '-1';
+```diff
+function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = false, updatedTransaction, isFromReviewDuplicates = false}: MoneyRequestViewProps) {
+-    const parentReportAction = parentReportActions?.[report?.parentReportActionID ?? '-1'];
++    const parentReportAction = parentReportActions?.[String(report?.parentReportActionID)];
 ```
 
 ### Extract complex types
diff --git a/src/CONST.ts b/src/CONST.ts
index ddf9ebad5b66..89276c51dd13 100755
--- a/src/CONST.ts
+++ b/src/CONST.ts
@@ -16,6 +16,11 @@ import type PlaidBankAccount from './types/onyx/PlaidBankAccount';
 const EMPTY_ARRAY = Object.freeze([]);
 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');
@@ -833,6 +838,8 @@ const CONST = {
     CLOUDFRONT_URL,
     EMPTY_ARRAY,
     EMPTY_OBJECT,
+    DEFAULT_NUMBER_ID,
+    DEFAULT_STRING_ID,
     USE_EXPENSIFY_URL,
     GOOGLE_MEET_URL_ANDROID: 'https://meet.google.com',
     GOOGLE_DOC_IMAGE_LINK_MATCH: 'googleusercontent.com',