Skip to content

Commit

Permalink
Merge branch 'main' into mkzie2-issue/51433
Browse files Browse the repository at this point in the history
  • Loading branch information
mkzie2 committed Nov 20, 2024
2 parents 9def46f + 9368566 commit f5082e8
Show file tree
Hide file tree
Showing 184 changed files with 3,732 additions and 2,026 deletions.
1 change: 1 addition & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ ONYX_METRICS=false
USE_THIRD_PARTY_SCRIPTS=false

EXPENSIFY_ACCOUNT_ID_ACCOUNTING=-1
EXPENSIFY_ACCOUNT_ID_ACCOUNTS_PAYABLE=-1
EXPENSIFY_ACCOUNT_ID_ADMIN=-1
EXPENSIFY_ACCOUNT_ID_BILLS=-1
EXPENSIFY_ACCOUNT_ID_CHRONOS=-1
Expand Down
2 changes: 0 additions & 2 deletions .github/workflows/buildAndroid.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ jobs:
build:
name: Build Android app
runs-on: ubuntu-latest-xl
env:
RUBYOPT: '-rostruct'
outputs:
AAB_FILE_NAME: ${{ steps.build.outputs.AAB_FILE_NAME }}
APK_FILE_NAME: ${{ steps.build.outputs.APK_FILE_NAME }}
Expand Down
6 changes: 0 additions & 6 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ jobs:
name: Upload Android build to Google Play Store
needs: buildAndroid
runs-on: ubuntu-latest
env:
RUBYOPT: '-rostruct'
steps:
- name: Checkout
uses: actions/checkout@v4
Expand Down Expand Up @@ -121,8 +119,6 @@ jobs:
needs: prep
if: ${{ github.ref == 'refs/heads/production' }}
runs-on: ubuntu-latest
env:
RUBYOPT: '-rostruct'
steps:
- name: Checkout
uses: actions/checkout@v4
Expand Down Expand Up @@ -170,8 +166,6 @@ jobs:
defaults:
run:
working-directory: Mobile-Expensify/react-native
env:
RUBYOPT: '-rostruct'
steps:
- name: Checkout
uses: actions/checkout@v4
Expand Down
2 changes: 0 additions & 2 deletions .github/workflows/testBuild.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,6 @@ jobs:
name: Upload Android app to S3
needs: [buildAndroid]
runs-on: ubuntu-latest
env:
RUBYOPT: '-rostruct'
outputs:
S3_APK_PATH: ${{ steps.exportS3Path.outputs.S3_APK_PATH }}
steps:
Expand Down
4 changes: 2 additions & 2 deletions android/app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ android {
minSdkVersion rootProject.ext.minSdkVersion
targetSdkVersion rootProject.ext.targetSdkVersion
multiDexEnabled rootProject.ext.multiDexEnabled
versionCode 1009006303
versionName "9.0.63-3"
versionCode 1009006402
versionName "9.0.64-2"
// Supported language variants must be declared here to avoid from being removed during the compilation.
// This also helps us to not include unnecessary language variants in the APK.
resConfigs "en", "es"
Expand Down
2 changes: 1 addition & 1 deletion contributingGuides/PERFORMANCE_METRICS.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Project is using Firebase for tracking these metrics. However, not all of them a
| `js_loaded` || The time it takes for the JavaScript bundle to load. <br><br>**Platforms:** Android, iOS | **Android:** Starts in the `onCreate` method.<br><br>**iOS:** Starts in the AppDelegate's `didFinishLaunchingWithOptions` method. | Stops at the first render of the app via native module on the JS side. |
| `_app_in_foreground` || The time when the app is running in the foreground and available to the user.<br><br>**Platforms:** Android, iOS | **Android:** Starts when the first activity to reach the foreground has its `onResume()` method called. <br><br>**iOS:** Starts when the application receives the `UIApplicationDidBecomeActiveNotification` notification. | **Android:** Stops when the last activity to leave the foreground has its `onStop()` method called. <br><br>**iOS:** Stops when it receives the `UIApplicationWillResignActiveNotification` notification. |
| `_app_in_background` || Time when the app is running in the background.<br><br>**Platforms:** Android, iOS | **Android:** Starts when the last activity to leave the foreground has its `onStop()` method called. <br><br>**iOS:** Starts when the application receives the `UIApplicationWillResignActiveNotification` notification. | **Android:** Stops when the first activity to reach the foreground has its `onResume()` method called. <br><br>**iOS:** Stops when it receives the `UIApplicationDidBecomeActiveNotification` notification. |
| `sidebar_loaded` | | Time taken for the Sidebar to load.<br><br>**Platforms:** All | Starts when the Sidebar is mounted. | Stops when the LHN finishes laying out. |
| `sidebar_loaded` | | Time taken for the Sidebar to load.<br><br>**Platforms:** All | Starts when the Sidebar is mounted. | Stops when the LHN finishes laying out. |
| `calc_most_recent_last_modified_action` || Time taken to find the most recently modified report action or report.<br><br>**Platforms:** All | Starts when the app reconnects to the network | Ends when the app reconnects to the network and the most recent report action or report is found. |
| `open_search` || Time taken to open up the Search Router.<br><br>**Platforms:** All | Starts when the Search Router icon in LHN is pressed. | Stops when the list of available options finishes laying out. |
| `load_search_options` || Time taken to generate the list of options used in the Search Router.<br><br>**Platforms:** All | Starts when the `getSearchOptions` function is called. | Stops when the list of available options is generated. |
Expand Down
2 changes: 1 addition & 1 deletion contributingGuides/STORYBOOK.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export {

That will give us an interactive playground to test out various component attributes with the defaults we passed.

![Storybook example](web/storybook-example.png)
![Storybook example](/web/storybook-example.png)

Note that we did not need to write any of the descriptions for these props. This is because they are automatically generated from a React component's `propTypes`.

Expand Down
64 changes: 56 additions & 8 deletions contributingGuides/STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -477,20 +477,68 @@ 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!**

> 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
const foo = report?.reportID ?? '';
const bar = report?.reportID ?? '0';

report ? report.reportID : '0';
report ? report.reportID : '';
const accountID = report?.ownerAccountID ?? -1;
const policyID = report?.policyID ?? '-1';
const managerID = report ? report.managerID : 0;

// GOOD
const foo = report?.reportID ?? '-1';
const accountID = report?.ownerAccountID ?? CONST.DEFAULT_NUMBER_ID;
const policyID = report?.policyID;
const managerID = report ? report.managerID : CONST.DEFAULT_NUMBER_ID;
```
Here are some common cases you may face when fixing your code to remove the old/bad 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 [parentReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`, {
canEvict: false,
});
- 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 simply use early return conditions here. Instead, we can check if `report?.parentReportActionID` is defined, if it is we can safely use it to find the record inside `parentReportActions`. If it's not defined, we just return `undefined`.
report ? report.reportID : '-1';
```diff
function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = false, updatedTransaction, isFromReviewDuplicates = false}: MoneyRequestViewProps) {
- const parentReportAction = parentReportActions?.[report?.parentReportActionID ?? '-1'];
+ const parentReportAction = report?.parentReportActionID ? parentReportActions?.[report.parentReportActionID] : undefined;
```
### Extract complex types
Expand Down
Loading

0 comments on commit f5082e8

Please sign in to comment.