Skip to content

Commit

Permalink
Unify DevLoadingView code on JS side to be platform-independent (#43992)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #43992

# Changelog:
[Internal] -

While looking into implementing native DevLoadingView on non-Android/iOS platform, I realized that the current code inside `LoadingView.android.js`/`LoadingView.ios.js` is functionally identical, and can be transformed into each other with simple code transformations.

This diff:
* Renames `LoadingView` into `DevLoadingView` (as it's arguably more fitting name, given that it also relies on `NativeDevLoadingView` native module implementation)
* Merges the iOS/Android specific JS files into one
* Factors usage of the colors out of the actual logic, to better separate presentation from the business logic

From the perspective of public APIs there should be no changes.

Reviewed By: christophpurrer

Differential Revision: D55914787

fbshipit-source-id: 656311db80e5ee03f60ee7ffcf5f405ca99a9ce5
  • Loading branch information
rshest authored and facebook-github-bot committed Apr 9, 2024
1 parent 1cb0a33 commit 881c0bc
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 106 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const loadingViewMock = {
hide: jest.fn(),
};
jest.mock(
'react-native/Libraries/Utilities/LoadingView',
'react-native/Libraries/Utilities/DevLoadingView',
() => loadingViewMock,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
*/

import Networking from '../../Network/RCTNetworking';
import DevLoadingView from '../../Utilities/DevLoadingView';
import HMRClient from '../../Utilities/HMRClient';
import LoadingView from '../../Utilities/LoadingView';
import getDevServer from './getDevServer';

declare var global: {globalEvalWithSourceUrl?: (string, string) => mixed, ...};
Expand Down Expand Up @@ -110,7 +110,7 @@ module.exports = function (bundlePathAndQuery: string): Promise<void> {
if (loadPromise) {
return loadPromise;
}
LoadingView.showMessage('Downloading...', 'load');
DevLoadingView.showMessage('Downloading...', 'load');
++pendingRequests;

loadPromise = asyncRequest(requestUrl)
Expand Down Expand Up @@ -143,7 +143,7 @@ module.exports = function (bundlePathAndQuery: string): Promise<void> {
})
.finally(() => {
if (!--pendingRequests) {
LoadingView.hide();
DevLoadingView.hide();
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,45 @@ import processColor from '../StyleSheet/processColor';
import Appearance from './Appearance';
import NativeDevLoadingView from './NativeDevLoadingView';

const COLOR_SCHEME = {
dark: {
refresh: {
backgroundColor: '#2584e8',
textColor: '#ffffff',
},
load: {
backgroundColor: '#fafafa',
textColor: '#242526',
},
},
default: {
refresh: {
backgroundColor: '#2584e8',
textColor: '#ffffff',
},
load: {
backgroundColor: '#404040',
textColor: '#ffffff',
},
},
};

module.exports = {
showMessage(message: string, type: 'load' | 'refresh') {
if (NativeDevLoadingView) {
const colorScheme =
Appearance.getColorScheme() === 'dark'
? COLOR_SCHEME.dark
: COLOR_SCHEME.default;

const colorSet = colorScheme[type];

let backgroundColor;
let textColor;

if (type === 'refresh') {
backgroundColor = processColor('#2584e8');
textColor = processColor('#ffffff');
} else if (type === 'load') {
if (Appearance.getColorScheme() === 'dark') {
backgroundColor = processColor('#fafafa');
textColor = processColor('#242526');
} else {
backgroundColor = processColor('#404040');
textColor = processColor('#ffffff');
}
if (colorSet) {
backgroundColor = processColor(colorSet.backgroundColor);
textColor = processColor(colorSet.textColor);
}

NativeDevLoadingView.showMessage(
Expand Down
16 changes: 8 additions & 8 deletions packages/react-native/Libraries/Utilities/HMRClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ const HMRClient: HMRClientNativeInterface = {
}

invariant(hmrClient, 'Expected HMRClient.setup() call at startup.');
const LoadingView = require('./LoadingView');
const DevLoadingView = require('./DevLoadingView');

// We use this for internal logging only.
// It doesn't affect the logic.
Expand All @@ -81,13 +81,13 @@ const HMRClient: HMRClientNativeInterface = {
const hasUpdates = hmrClient.hasPendingUpdates();

if (hasUpdates) {
LoadingView.showMessage('Refreshing...', 'refresh');
DevLoadingView.showMessage('Refreshing...', 'refresh');
}
try {
hmrClient.enable();
} finally {
if (hasUpdates) {
LoadingView.hide();
DevLoadingView.hide();
}
}

Expand Down Expand Up @@ -181,7 +181,7 @@ const HMRClient: HMRClientNativeInterface = {
invariant(!hmrClient, 'Cannot initialize hmrClient twice');

// Moving to top gives errors due to NativeModules not being initialized
const LoadingView = require('./LoadingView');
const DevLoadingView = require('./DevLoadingView');

const serverHost = port !== null && port !== '' ? `${host}:${port}` : host;

Expand Down Expand Up @@ -230,7 +230,7 @@ Error: ${e.message}`;
didConnect = true;

if (client.isEnabled() && !isInitialUpdate) {
LoadingView.showMessage('Refreshing...', 'refresh');
DevLoadingView.showMessage('Refreshing...', 'refresh');
}
});

Expand All @@ -242,11 +242,11 @@ Error: ${e.message}`;
});

client.on('update-done', () => {
LoadingView.hide();
DevLoadingView.hide();
});

client.on('error', data => {
LoadingView.hide();
DevLoadingView.hide();

if (data.type === 'GraphNotFoundError') {
client.close();
Expand All @@ -267,7 +267,7 @@ Error: ${e.message}`;
});

client.on('close', closeEvent => {
LoadingView.hide();
DevLoadingView.hide();

// https://www.rfc-editor.org/rfc/rfc6455.html#section-7.4.1
// https://www.rfc-editor.org/rfc/rfc6455.html#section-7.1.5
Expand Down
50 changes: 0 additions & 50 deletions packages/react-native/Libraries/Utilities/LoadingView.ios.js

This file was deleted.

16 changes: 0 additions & 16 deletions packages/react-native/Libraries/Utilities/LoadingView.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8516,23 +8516,7 @@ export interface IPerformanceLogger {
"
`;

exports[`public API should not change unintentionally Libraries/Utilities/LoadingView.android.js 1`] = `
"declare module.exports: {
showMessage(message: string, type: \\"load\\" | \\"refresh\\"): void,
hide(): void,
};
"
`;

exports[`public API should not change unintentionally Libraries/Utilities/LoadingView.ios.js 1`] = `
"declare module.exports: {
showMessage(message: string, type: \\"load\\" | \\"refresh\\"): void,
hide(): void,
};
"
`;

exports[`public API should not change unintentionally Libraries/Utilities/LoadingView.js 1`] = `
exports[`public API should not change unintentionally Libraries/Utilities/DevLoadingView.js 1`] = `
"declare module.exports: {
showMessage(message: string, type: \\"load\\" | \\"refresh\\"): void,
hide(): void,
Expand Down

0 comments on commit 881c0bc

Please sign in to comment.