[No QA] Revert "[No QA] Fix import/no-cycle - part 1 - follow up"

.storybook/preview.tsx

@@ -1,10 +1,9 @@
+import '@libs/Middleware/register';
 import EnvironmentProvider from '@components/EnvironmentContextProvider';
 import OnyxListItemProvider from '@components/OnyxListItemProvider';
 import ScreenWrapperStatusContext from '@components/ScreenWrapper/ScreenWrapperStatusContext';
 import {SearchContextProvider} from '@components/Search/SearchContextProvider';
 
-import registerMiddlewares from '@libs/Middleware/register';
-
 import colors from '@styles/theme/colors';
 
 import ComposeProviders from '@src/components/ComposeProviders';
@@ -24,8 +23,6 @@ import {SafeAreaProvider} from 'react-native-safe-area-context';
 
 import './fonts.css';
 
-registerMiddlewares();
-
 Onyx.init({
     keys: ONYXKEYS,
 });

src/libs/Middleware/register.ts

@@ -24,57 +24,48 @@ import {
 // This lives here rather than in libs/API because six of the middlewares below import user actions, and every
 // user action imports libs/API. Registering from inside libs/API therefore closes an import cycle: libs/API
 // imports Middleware, Middleware imports an action, and that action imports libs/API again. Instead the
-// composition root calls this explicitly, before anything can call processWithMiddleware: see src/setup/index.ts.
-let hasRegistered = false;
+// composition root imports this module for its side effect, before anything can call processWithMiddleware:
+// see src/setup/index.ts.
 
-function registerMiddlewares() {
-    if (hasRegistered) {
-        return;
-    }
-    hasRegistered = true;
+// Logging - Logs request details and errors.
+addMiddleware(Logging);
 
-    // Logging - Logs request details and errors.
-    addMiddleware(Logging);
+// Duplicates API calls (tagged with mockRequest=true) when the server sends load-test parameters via the X-Load-Test response header.
+addMiddleware(LoadTest);
 
-    // Duplicates API calls (tagged with mockRequest=true) when the server sends load-test parameters via the X-Load-Test response header.
-    addMiddleware(LoadTest);
+// FailureTracking - Observes request outcomes and feeds them to FailureTracker for sustained failure detection.
+addMiddleware(FailureTracking);
 
-    // FailureTracking - Observes request outcomes and feeds them to FailureTracker for sustained failure detection.
-    addMiddleware(FailureTracking);
+// Reauthentication - Handles jsonCode 407 which indicates an expired authToken. We need to reauthenticate and get a new authToken with our stored credentials.
+addMiddleware(Reauthentication);
 
-    // Reauthentication - Handles jsonCode 407 which indicates an expired authToken. We need to reauthenticate and get a new authToken with our stored credentials.
-    addMiddleware(Reauthentication);
+// Handles the case when the copilot has been deleted. The response contains jsonCode 408 and a message indicating account deletion
+addMiddleware(handleDeletedAccount);
 
-    // Handles the case when the copilot has been deleted. The response contains jsonCode 408 and a message indicating account deletion
-    addMiddleware(handleDeletedAccount);
+// Handle supportal permission denial centrally
+addMiddleware(SupportalPermission);
 
-    // Handle supportal permission denial centrally
-    addMiddleware(SupportalPermission);
+// If an optimistic ID is not used by the server, this will update the remaining serialized requests using that optimistic ID to use the correct ID instead.
+addMiddleware(HandleUnusedOptimisticID);
 
-    // If an optimistic ID is not used by the server, this will update the remaining serialized requests using that optimistic ID to use the correct ID instead.
-    addMiddleware(HandleUnusedOptimisticID);
+addMiddleware(Pagination);
 
-    addMiddleware(Pagination);
+// SentryServerTiming - Tracks server round-trip time for configured command groups via Sentry spans.
+addMiddleware(SentryServerTiming);
 
-    // SentryServerTiming - Tracks server round-trip time for configured command groups via Sentry spans.
-    addMiddleware(SentryServerTiming);
+// RecordFullReconnectTime - Records the full-reconnect time into an OpenApp/full-ReconnectApp response. Must run before SaveResponseInOnyx applies the response.
+addMiddleware(RecordFullReconnectTime);
 
-    // RecordFullReconnectTime - Records the full-reconnect time into an OpenApp/full-ReconnectApp response. Must run before SaveResponseInOnyx applies the response.
-    addMiddleware(RecordFullReconnectTime);
+// LoadPostDataForOpenOrReconnect - Sends the reads that OpenApp/ReconnectApp does not return, once per response that reaches the server.
+addMiddleware(LoadPostDataForOpenOrReconnect);
 
-    // LoadPostDataForOpenOrReconnect - Sends the reads that OpenApp/ReconnectApp does not return, once per response that reaches the server.
-    addMiddleware(LoadPostDataForOpenOrReconnect);
+// HandleMovedScanFailedExpenses - Retires the optimistic report built for scan-failed expenses moved on payment once the backend answers
+// with the report it created for them. Must run before SaveResponseInOnyx so its updates are applied with the response.
+addMiddleware(HandleMovedScanFailedExpenses);
 
-    // HandleMovedScanFailedExpenses - Retires the optimistic report built for scan-failed expenses moved on payment once the backend answers
-    // with the report it created for them. Must run before SaveResponseInOnyx so its updates are applied with the response.
-    addMiddleware(HandleMovedScanFailedExpenses);
+// SaveResponseInOnyx - Merges either the successData or failureData (or finallyData, if included in place of the former two values) into Onyx depending on if the call was successful or not. This must be the last middleware that applies Onyx data
+// (middlewares after it, like FraudMonitoring, must not write Onyx), because the SequentialQueue depends on the result of this middleware to pause the queue (if needed) to bring the app to an up-to-date state.
+addMiddleware(SaveResponseInOnyx);
 
-    // SaveResponseInOnyx - Merges either the successData or failureData (or finallyData, if included in place of the former two values) into Onyx depending on if the call was successful or not. This must be the last middleware that applies Onyx data
-    // (middlewares after it, like FraudMonitoring, must not write Onyx), because the SequentialQueue depends on the result of this middleware to pause the queue (if needed) to bring the app to an up-to-date state.
-    addMiddleware(SaveResponseInOnyx);
-
-    // FraudMonitoring - Tags the request with the appropriate Fraud Protection event.
-    addMiddleware(FraudMonitoring);
-}
-
-export default registerMiddlewares;
+// FraudMonitoring - Tags the request with the appropriate Fraud Protection event.
+addMiddleware(FraudMonitoring);

src/setup/index.ts

@@ -1,6 +1,6 @@
+import '@libs/Middleware/register';
 import {finishCloudflareSignInFromURL} from '@libs/CloudflareAccess/finishSignInFromURL';
 import intlPolyfill from '@libs/IntlPolyfill';
-import registerMiddlewares from '@libs/Middleware/register';
 import registerReportActionsPagination from '@libs/registerReportActionsPagination';
 
 import {setDeviceID} from '@userActions/Device';
@@ -21,8 +21,6 @@ import telemetry from './telemetry';
 const enableDevTools = Config?.USE_REDUX_DEVTOOLS === 'true';
 
 export default function () {
-    registerMiddlewares();
-
     telemetry();
 
     toSortedPolyfill.shim();

tests/ui/SearchPageTest.tsx

@@ -1,3 +1,4 @@
+import '@libs/Middleware/register';
 import {act, render, screen} from '@testing-library/react-native';
 
 import ComposeProviders from '@components/ComposeProviders';
@@ -13,7 +14,6 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout';
 
 import {search} from '@libs/actions/Search';
 import type * as SearchActions from '@libs/actions/Search';
-import registerMiddlewares from '@libs/Middleware/register';
 import createRootStackNavigator from '@libs/Navigation/AppNavigator/createRootStackNavigator';
 import navigationRef from '@libs/Navigation/navigationRef';
 import createPlatformStackNavigator from '@libs/Navigation/PlatformStackNavigation/createPlatformStackNavigator';
@@ -39,8 +39,6 @@ import Onyx from 'react-native-onyx';
 
 import createMock from '../utils/createMock';
 
-registerMiddlewares();
-
 jest.mock('@hooks/useResponsiveLayout', () => jest.fn());
 jest.mock('@hooks/useNetwork', () => jest.fn());
 const mockSearchQueryParam = jest.fn(() => 'type:chat category:abcd');

tests/unit/MiddlewareEntryPointTest.ts

@@ -1,26 +1,16 @@
 import type * as RequestModule from '@libs/Request';
 import {addMiddleware} from '@libs/Request';
 
-import appSetup from '@src/setup';
-
 jest.mock('@libs/Request', () => ({
     ...jest.requireActual<typeof RequestModule>('@libs/Request'),
     addMiddleware: jest.fn(),
 }));
 
-jest.mock('@libs/registerReportActionsPagination', () => jest.fn());
-jest.mock('@libs/IntlPolyfill', () => jest.fn());
-jest.mock('@userActions/Device', () => ({setDeviceID: jest.fn()}));
-jest.mock('@userActions/OnyxDerived', () => jest.fn());
-jest.mock('@src/setup/addUtilsToWindow', () => jest.fn());
-jest.mock('@src/setup/platformSetup', () => jest.fn());
-jest.mock('@src/setup/telemetry', () => jest.fn());
-
 describe('src/setup attaches the API middlewares', () => {
-    it('registers all 14 middlewares when the composition root runs', () => {
+    it('registers all 14 middlewares at module scope when the composition root loads', () => {
         expect(jest.mocked(addMiddleware)).not.toHaveBeenCalled();
 
-        appSetup();
+        require('@src/setup');
 
         expect(jest.mocked(addMiddleware)).toHaveBeenCalledTimes(14);
     });

tests/unit/MiddlewareRegistrationTest.ts

@@ -14,7 +14,6 @@ import {
     SentryServerTiming,
     SupportalPermission,
 } from '@libs/Middleware';
-import registerMiddlewares from '@libs/Middleware/register';
 import type * as RequestModule from '@libs/Request';
 import {addMiddleware} from '@libs/Request';
 
@@ -44,7 +43,9 @@ describe('Middleware registration', () => {
     let registered: RequestModule.Middleware[] = [];
 
     beforeAll(() => {
-        registerMiddlewares();
+        // jest.isolateModules would give register.ts its own module registry, so the middlewares it resolves
+        // would be distinct function objects from the ones imported above and every identity check would fail.
+        require('@libs/Middleware/register');
         registered = jest.mocked(addMiddleware).mock.calls.map(([middleware]) => middleware);
     });
 

tests/unit/TransactionGroupListItemTest.tsx

@@ -1,3 +1,4 @@
+import '@libs/Middleware/register';
 import {fireEvent, render, screen} from '@testing-library/react-native';
 
 import ComposeProviders from '@components/ComposeProviders';
@@ -12,7 +13,6 @@ import type {
     TransactionReportGroupListItemType,
 } from '@components/Search/SearchList/ListItem/types';
 
-import registerMiddlewares from '@libs/Middleware/register';
 import {buildSearchQueryJSON} from '@libs/SearchQueryUtils';
 
 import TransactionGroupListItem from '@src/components/Search/SearchList/ListItem/TransactionGroupListItem';
@@ -29,8 +29,6 @@ import type * as MockUsePaymentContextUtil from '../utils/mockUsePaymentContext'
 
 import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct';
 
-registerMiddlewares();
-
 jest.mock('@libs/actions/Search', () => ({
     search: jest.fn(),
     handleActionButtonPress: jest.fn(),

tests/utils/TestHelper.ts

@@ -4,7 +4,6 @@ import type {ApiCommand, ApiRequestCommandParameters} from '@libs/API/types';
 import {convertToFrontendAmountAsInteger, sanitizeCurrencyCode} from '@libs/CurrencyUtils';
 import {formatPhoneNumberWithCountryCode} from '@libs/LocalePhoneNumber';
 import {translate} from '@libs/Localize';
-import registerMiddlewares from '@libs/Middleware/register';
 import {format as formatNumber} from '@libs/NumberFormatUtils';
 import Pusher from '@libs/Pusher';
 import PusherConnectionManager from '@libs/PusherConnectionManager';
@@ -34,10 +33,6 @@ import {isObject} from './typeGuards';
 import waitForBatchedUpdates from './waitForBatchedUpdates';
 import waitForBatchedUpdatesWithAct from './waitForBatchedUpdatesWithAct';
 
-// Every test that imports this module can make an API call without first calling setupApp(), so middlewares
-// must be registered here too. Idempotent, so this doesn't conflict with the appSetup() call inside setupApp().
-registerMiddlewares();
-
 type MockFetch = jest.Mock<ReturnType<typeof fetch>, Parameters<typeof fetch>> & {
     pause: () => void;
     fail: () => void;