From 2707d4a55a1f12fb44885299f90d5d49ca342f5e Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Tue, 10 Sep 2024 14:11:51 +0200 Subject: [PATCH 01/11] Create Incremental 'withOnyx' migration CI check --- .github/scripts/withOnyxMigration.sh | 59 +++++++++++++++++++++++++ .github/workflows/withOnyxMigration.yml | 20 +++++++++ 2 files changed, 79 insertions(+) create mode 100755 .github/scripts/withOnyxMigration.sh create mode 100644 .github/workflows/withOnyxMigration.yml diff --git a/.github/scripts/withOnyxMigration.sh b/.github/scripts/withOnyxMigration.sh new file mode 100755 index 0000000000000..6ec741aefdee9 --- /dev/null +++ b/.github/scripts/withOnyxMigration.sh @@ -0,0 +1,59 @@ +#!/bin/bash + +RED='\033[0;31m' +ORANGE='\033[0;33m' +GREEN='\033[0;32m' +BOLD_WHITE='\033[1;37m' + +# Set IFS to newline to properly handle file names with spaces. +# This makes 'for' loop treat each line as a separate item. +IFS=$'\n' + +# Fetch the main branch +git fetch origin main --no-tags --depth=1 + +# Get the list of modified and added files +modified_files=$(git diff --name-only --diff-filter=AM origin/main HEAD -- '*.ts' '*.tsx') + +# Count instances of 'withOnyx' on main branch and current branch +count_main=0 +count_current=0 + +echo -e "\nWe're gradually moving from 'withOnyx' to 'useOnyx'. 'useOnyx' hook simplifies the code, offers performance benefits and improves TypeScript compilation time." +echo -e "For more details, see https://expensify.slack.com/archives/C01GTK53T8Q/p1725905735105989.\n" +echo -e "${BOLD_WHITE}If you are blocked by this on something that is very urgent, you can always ignore this check and merge with it failing.\n" + +if [ -z "$modified_files" ]; then + echo -e "${GREEN}No changes detected. Exiting." + exit 0 +fi + +for file in $modified_files; do + # 2>/dev/null is used to ignore errors when the file doesn't exist in the main branch + count_main_file=$(git show origin/main:"$file" 2>/dev/null | grep -o 'withOnyx<' | wc -l) + count_current_file=$(grep -o 'withOnyx<' "$file" | wc -l) + + count_main=$((count_main + count_main_file)) + count_current=$((count_current + count_current_file)) + + if ((count_current_file > 0 && count_current_file >= count_main_file)); then + echo -e "${ORANGE}WARNING: '$file' was modified on this branch and still contains 'withOnyx' usages. Consider replacing it with 'useOnyx' hooks." + fi +done + +if ((count_current == 0 && count_main == 0)); then + echo -e "${GREEN}No changes detected. Exiting." + exit 0 +fi + +if ((count_current == count_main)); then + echo -e "${RED}\nERROR: There's no reduction of 'withOnyx' usages on the current branch compared to main. Please migrate one of the above files to use 'useOnyx' hooks instead." + exit 1 +fi + +if ((count_current > count_main)); then + echo -e "${RED}\nERROR: There's an increase in 'withOnyx' usages on the current branch compared to main. Please migrate it to use 'useOnyx' hooks instead." + exit 1 +fi + +echo -e "${GREEN}\nIn the modified files, the usage of 'withOnyx' has been reduced from $count_main to $count_current. Great job!" diff --git a/.github/workflows/withOnyxMigration.yml b/.github/workflows/withOnyxMigration.yml new file mode 100644 index 0000000000000..789d26da03d1f --- /dev/null +++ b/.github/workflows/withOnyxMigration.yml @@ -0,0 +1,20 @@ +name: Incremental 'withOnyx' migration + +on: + workflow_call: + pull_request: + types: [opened, synchronize] + branches-ignore: [staging, production] + paths: ['**.ts', '**.tsx'] + +jobs: + withOnyxMigration: + if: ${{ github.actor != 'OSBotify' || github.event_name == 'workflow_call' }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: ./.github/actions/composite/setupNode + + - name: Check for 'withOnyx' usages in modified files + run: ./.github/scripts/withOnyxMigration.sh From b98527d94d69f8939daa1b39109e3b0adad54f1e Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Tue, 10 Sep 2024 14:12:56 +0200 Subject: [PATCH 02/11] Create a file without withOnyx --- src/test.ts | 2 ++ 1 file changed, 2 insertions(+) create mode 100755 src/test.ts diff --git a/src/test.ts b/src/test.ts new file mode 100755 index 0000000000000..84dadc7ac5c30 --- /dev/null +++ b/src/test.ts @@ -0,0 +1,2 @@ +/* eslint-disable no-console */ +console.log('CONST'); From 2ffd307c91561e233267d55e304f9b8a78ce94e4 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Tue, 10 Sep 2024 14:13:22 +0200 Subject: [PATCH 03/11] Create a file with withOnyx --- src/test.ts | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/test.ts b/src/test.ts index 84dadc7ac5c30..d639533c8dcf6 100755 --- a/src/test.ts +++ b/src/test.ts @@ -1,2 +1,30 @@ /* eslint-disable no-console */ console.log('CONST'); + +export default withOnyx({ + isCheckingPublicRoom: { + key: ONYXKEYS.IS_CHECKING_PUBLIC_ROOM, + initWithStoredValues: false, + }, + updateAvailable: { + key: ONYXKEYS.UPDATE_AVAILABLE, + initWithStoredValues: false, + }, + updateRequired: { + key: ONYXKEYS.UPDATE_REQUIRED, + initWithStoredValues: false, + }, + isSidebarLoaded: { + key: ONYXKEYS.IS_SIDEBAR_LOADED, + }, + screenShareRequest: { + key: ONYXKEYS.SCREEN_SHARE_REQUEST, + }, + focusModeNotification: { + key: ONYXKEYS.FOCUS_MODE_NOTIFICATION, + initWithStoredValues: false, + }, + lastVisitedPath: { + key: ONYXKEYS.LAST_VISITED_PATH, + }, +})(Expensify); From 7643c7d3b767f00c5a9983402c91e22d805d905e Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Tue, 10 Sep 2024 14:14:08 +0200 Subject: [PATCH 04/11] Edit existing file without migrating to useOnyx --- src/Expensify.tsx | 2 +- src/test.ts | 30 ------------------------------ 2 files changed, 1 insertion(+), 31 deletions(-) delete mode 100755 src/test.ts diff --git a/src/Expensify.tsx b/src/Expensify.tsx index 62e7839b21f0f..40718f2be8b29 100644 --- a/src/Expensify.tsx +++ b/src/Expensify.tsx @@ -312,7 +312,7 @@ function Expensify({ ); } -Expensify.displayName = 'Expensify'; +Expensify.displayName = 'Expensify2'; export default withOnyx({ isCheckingPublicRoom: { diff --git a/src/test.ts b/src/test.ts deleted file mode 100755 index d639533c8dcf6..0000000000000 --- a/src/test.ts +++ /dev/null @@ -1,30 +0,0 @@ -/* eslint-disable no-console */ -console.log('CONST'); - -export default withOnyx({ - isCheckingPublicRoom: { - key: ONYXKEYS.IS_CHECKING_PUBLIC_ROOM, - initWithStoredValues: false, - }, - updateAvailable: { - key: ONYXKEYS.UPDATE_AVAILABLE, - initWithStoredValues: false, - }, - updateRequired: { - key: ONYXKEYS.UPDATE_REQUIRED, - initWithStoredValues: false, - }, - isSidebarLoaded: { - key: ONYXKEYS.IS_SIDEBAR_LOADED, - }, - screenShareRequest: { - key: ONYXKEYS.SCREEN_SHARE_REQUEST, - }, - focusModeNotification: { - key: ONYXKEYS.FOCUS_MODE_NOTIFICATION, - initWithStoredValues: false, - }, - lastVisitedPath: { - key: ONYXKEYS.LAST_VISITED_PATH, - }, -})(Expensify); From 6f2646e4a8b45369d69a234500ac257440e5d8ef Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Tue, 10 Sep 2024 14:14:28 +0200 Subject: [PATCH 05/11] Edit existing file and migrate to useOnyx --- src/Expensify.tsx | 30 ++---------------------------- 1 file changed, 2 insertions(+), 28 deletions(-) diff --git a/src/Expensify.tsx b/src/Expensify.tsx index 40718f2be8b29..1fcb947faff6b 100644 --- a/src/Expensify.tsx +++ b/src/Expensify.tsx @@ -3,7 +3,7 @@ import React, {useCallback, useContext, useEffect, useLayoutEffect, useMemo, use import type {NativeEventSubscription} from 'react-native'; import {AppState, Linking, NativeModules, Platform} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; -import Onyx, {useOnyx, withOnyx} from 'react-native-onyx'; +import Onyx, {useOnyx} from 'react-native-onyx'; import ConfirmModal from './components/ConfirmModal'; import DeeplinkWrapper from './components/DeeplinkWrapper'; import EmojiPicker from './components/EmojiPicker/EmojiPicker'; @@ -314,30 +314,4 @@ function Expensify({ Expensify.displayName = 'Expensify2'; -export default withOnyx({ - isCheckingPublicRoom: { - key: ONYXKEYS.IS_CHECKING_PUBLIC_ROOM, - initWithStoredValues: false, - }, - updateAvailable: { - key: ONYXKEYS.UPDATE_AVAILABLE, - initWithStoredValues: false, - }, - updateRequired: { - key: ONYXKEYS.UPDATE_REQUIRED, - initWithStoredValues: false, - }, - isSidebarLoaded: { - key: ONYXKEYS.IS_SIDEBAR_LOADED, - }, - screenShareRequest: { - key: ONYXKEYS.SCREEN_SHARE_REQUEST, - }, - focusModeNotification: { - key: ONYXKEYS.FOCUS_MODE_NOTIFICATION, - initWithStoredValues: false, - }, - lastVisitedPath: { - key: ONYXKEYS.LAST_VISITED_PATH, - }, -})(Expensify); +export default Expensify; From bec270a4d488de44fa7bf1c5bc24829c45f67985 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Tue, 10 Sep 2024 14:14:51 +0200 Subject: [PATCH 06/11] No modifications, should not even trigger the check --- src/Expensify.tsx | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/src/Expensify.tsx b/src/Expensify.tsx index 1fcb947faff6b..62e7839b21f0f 100644 --- a/src/Expensify.tsx +++ b/src/Expensify.tsx @@ -3,7 +3,7 @@ import React, {useCallback, useContext, useEffect, useLayoutEffect, useMemo, use import type {NativeEventSubscription} from 'react-native'; import {AppState, Linking, NativeModules, Platform} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; -import Onyx, {useOnyx} from 'react-native-onyx'; +import Onyx, {useOnyx, withOnyx} from 'react-native-onyx'; import ConfirmModal from './components/ConfirmModal'; import DeeplinkWrapper from './components/DeeplinkWrapper'; import EmojiPicker from './components/EmojiPicker/EmojiPicker'; @@ -312,6 +312,32 @@ function Expensify({ ); } -Expensify.displayName = 'Expensify2'; - -export default Expensify; +Expensify.displayName = 'Expensify'; + +export default withOnyx({ + isCheckingPublicRoom: { + key: ONYXKEYS.IS_CHECKING_PUBLIC_ROOM, + initWithStoredValues: false, + }, + updateAvailable: { + key: ONYXKEYS.UPDATE_AVAILABLE, + initWithStoredValues: false, + }, + updateRequired: { + key: ONYXKEYS.UPDATE_REQUIRED, + initWithStoredValues: false, + }, + isSidebarLoaded: { + key: ONYXKEYS.IS_SIDEBAR_LOADED, + }, + screenShareRequest: { + key: ONYXKEYS.SCREEN_SHARE_REQUEST, + }, + focusModeNotification: { + key: ONYXKEYS.FOCUS_MODE_NOTIFICATION, + initWithStoredValues: false, + }, + lastVisitedPath: { + key: ONYXKEYS.LAST_VISITED_PATH, + }, +})(Expensify); From 19960bdc22a49495254bf2e23f59f99c353273b2 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Tue, 10 Sep 2024 16:55:31 +0200 Subject: [PATCH 07/11] Test changing multiple files --- src/components/AddPaymentMethodMenu.tsx | 2 +- src/components/AddPlaidBankAccount.tsx | 2 +- src/components/AnonymousReportFooter.tsx | 2 +- src/components/ArchivedReportFooter.tsx | 13 ++----------- 4 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/components/AddPaymentMethodMenu.tsx b/src/components/AddPaymentMethodMenu.tsx index 5621c031f9598..a5f23384cc22a 100644 --- a/src/components/AddPaymentMethodMenu.tsx +++ b/src/components/AddPaymentMethodMenu.tsx @@ -138,7 +138,7 @@ function AddPaymentMethodMenu({ ); } -AddPaymentMethodMenu.displayName = 'AddPaymentMethodMenu'; +AddPaymentMethodMenu.displayName = 'AddPaymentMethodMenu2'; export default withOnyx({ session: { diff --git a/src/components/AddPlaidBankAccount.tsx b/src/components/AddPlaidBankAccount.tsx index 4de286183ea85..5d699f26a5af3 100644 --- a/src/components/AddPlaidBankAccount.tsx +++ b/src/components/AddPlaidBankAccount.tsx @@ -285,7 +285,7 @@ function AddPlaidBankAccount({ ); } -AddPlaidBankAccount.displayName = 'AddPlaidBankAccount'; +AddPlaidBankAccount.displayName = 'AddPlaidBankAccount2'; export default withOnyx({ plaidLinkToken: { diff --git a/src/components/AnonymousReportFooter.tsx b/src/components/AnonymousReportFooter.tsx index 078b850de5ff7..2e285e4c1457f 100644 --- a/src/components/AnonymousReportFooter.tsx +++ b/src/components/AnonymousReportFooter.tsx @@ -59,7 +59,7 @@ function AnonymousReportFooter({isSmallSizeLayout = false, report, policy}: Anon ); } -AnonymousReportFooter.displayName = 'AnonymousReportFooter'; +AnonymousReportFooter.displayName = 'AnonymousReportFooter2'; export default withOnyx({ policy: { diff --git a/src/components/ArchivedReportFooter.tsx b/src/components/ArchivedReportFooter.tsx index 859d59278cdd6..cdc5dd947ada6 100644 --- a/src/components/ArchivedReportFooter.tsx +++ b/src/components/ArchivedReportFooter.tsx @@ -72,15 +72,6 @@ function ArchivedReportFooter({report, reportClosedAction, personalDetails = {}} ); } -ArchivedReportFooter.displayName = 'ArchivedReportFooter'; +ArchivedReportFooter.displayName = 'ArchivedReportFooter2'; -export default withOnyx({ - personalDetails: { - key: ONYXKEYS.PERSONAL_DETAILS_LIST, - }, - reportClosedAction: { - key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`, - canEvict: false, - selector: ReportActionsUtils.getLastClosedReportAction, - }, -})(ArchivedReportFooter); +export default ArchivedReportFooter; From f69c458ac28fc3e53a0b4bd54c32faa18bb0173b Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Tue, 10 Sep 2024 16:58:53 +0200 Subject: [PATCH 08/11] Test changing multiple files v2 --- src/components/ArchivedReportFooter.tsx | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/components/ArchivedReportFooter.tsx b/src/components/ArchivedReportFooter.tsx index cdc5dd947ada6..859d59278cdd6 100644 --- a/src/components/ArchivedReportFooter.tsx +++ b/src/components/ArchivedReportFooter.tsx @@ -72,6 +72,15 @@ function ArchivedReportFooter({report, reportClosedAction, personalDetails = {}} ); } -ArchivedReportFooter.displayName = 'ArchivedReportFooter2'; +ArchivedReportFooter.displayName = 'ArchivedReportFooter'; -export default ArchivedReportFooter; +export default withOnyx({ + personalDetails: { + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + }, + reportClosedAction: { + key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`, + canEvict: false, + selector: ReportActionsUtils.getLastClosedReportAction, + }, +})(ArchivedReportFooter); From 2f88df3da63086b97d8e350f171c1e7c6fb6e04c Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Tue, 10 Sep 2024 16:59:55 +0200 Subject: [PATCH 09/11] Revert tests cases --- src/components/AddPaymentMethodMenu.tsx | 2 +- src/components/AddPlaidBankAccount.tsx | 2 +- src/components/AnonymousReportFooter.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/AddPaymentMethodMenu.tsx b/src/components/AddPaymentMethodMenu.tsx index a5f23384cc22a..5621c031f9598 100644 --- a/src/components/AddPaymentMethodMenu.tsx +++ b/src/components/AddPaymentMethodMenu.tsx @@ -138,7 +138,7 @@ function AddPaymentMethodMenu({ ); } -AddPaymentMethodMenu.displayName = 'AddPaymentMethodMenu2'; +AddPaymentMethodMenu.displayName = 'AddPaymentMethodMenu'; export default withOnyx({ session: { diff --git a/src/components/AddPlaidBankAccount.tsx b/src/components/AddPlaidBankAccount.tsx index 5d699f26a5af3..4de286183ea85 100644 --- a/src/components/AddPlaidBankAccount.tsx +++ b/src/components/AddPlaidBankAccount.tsx @@ -285,7 +285,7 @@ function AddPlaidBankAccount({ ); } -AddPlaidBankAccount.displayName = 'AddPlaidBankAccount2'; +AddPlaidBankAccount.displayName = 'AddPlaidBankAccount'; export default withOnyx({ plaidLinkToken: { diff --git a/src/components/AnonymousReportFooter.tsx b/src/components/AnonymousReportFooter.tsx index 2e285e4c1457f..078b850de5ff7 100644 --- a/src/components/AnonymousReportFooter.tsx +++ b/src/components/AnonymousReportFooter.tsx @@ -59,7 +59,7 @@ function AnonymousReportFooter({isSmallSizeLayout = false, report, policy}: Anon ); } -AnonymousReportFooter.displayName = 'AnonymousReportFooter2'; +AnonymousReportFooter.displayName = 'AnonymousReportFooter'; export default withOnyx({ policy: { From e1f6100e6f1358d800cc2a98935bc4508f6a6485 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Wed, 11 Sep 2024 14:49:18 +0200 Subject: [PATCH 10/11] Adjust the PR after feedback --- .github/scripts/withOnyxMigration.sh | 2 +- .github/workflows/withOnyxMigration.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/scripts/withOnyxMigration.sh b/.github/scripts/withOnyxMigration.sh index 6ec741aefdee9..cd10017a8cc48 100755 --- a/.github/scripts/withOnyxMigration.sh +++ b/.github/scripts/withOnyxMigration.sh @@ -13,7 +13,7 @@ IFS=$'\n' git fetch origin main --no-tags --depth=1 # Get the list of modified and added files -modified_files=$(git diff --name-only --diff-filter=AM origin/main HEAD -- '*.ts' '*.tsx') +modified_files=$(git diff --name-only --diff-filter=AM origin/main HEAD -- '*.tsx') # Count instances of 'withOnyx' on main branch and current branch count_main=0 diff --git a/.github/workflows/withOnyxMigration.yml b/.github/workflows/withOnyxMigration.yml index 789d26da03d1f..8d688e195366c 100644 --- a/.github/workflows/withOnyxMigration.yml +++ b/.github/workflows/withOnyxMigration.yml @@ -5,7 +5,7 @@ on: pull_request: types: [opened, synchronize] branches-ignore: [staging, production] - paths: ['**.ts', '**.tsx'] + paths: ['**.ts'] jobs: withOnyxMigration: From 3b6ee28fe25d9adf0fc28bcad784d539e5bb4881 Mon Sep 17 00:00:00 2001 From: Blazej Kustra Date: Wed, 11 Sep 2024 14:55:44 +0200 Subject: [PATCH 11/11] Adjust an echo --- .github/scripts/withOnyxMigration.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/withOnyxMigration.sh b/.github/scripts/withOnyxMigration.sh index cd10017a8cc48..beee6a5712825 100755 --- a/.github/scripts/withOnyxMigration.sh +++ b/.github/scripts/withOnyxMigration.sh @@ -21,7 +21,7 @@ count_current=0 echo -e "\nWe're gradually moving from 'withOnyx' to 'useOnyx'. 'useOnyx' hook simplifies the code, offers performance benefits and improves TypeScript compilation time." echo -e "For more details, see https://expensify.slack.com/archives/C01GTK53T8Q/p1725905735105989.\n" -echo -e "${BOLD_WHITE}If you are blocked by this on something that is very urgent, you can always ignore this check and merge with it failing.\n" +echo -e "${BOLD_WHITE}If this check is blocking you on something that is very urgent, you can always ignore it.\n" if [ -z "$modified_files" ]; then echo -e "${GREEN}No changes detected. Exiting."