Skip to content

Commit

Permalink
Remove logged errors for feature flags not implemented in native when…
Browse files Browse the repository at this point in the history
… skipNativeAPI is used (facebook#49050)

Summary:
Pull Request resolved: facebook#49050

Changelog: [internal]

We added support for feature flags that don't have a native module definition so we could handle cases where the JS changes progressed faster than native ones, but we recently saw that when native catches up, the API starts logging an error through `console.error` about the native module method not being available.

That's an expected result of this feature and it's when we can clean up the code in JS, so we shouldn't be logging errors in that case.

This removes the error for them specifically.

Reviewed By: elicwhite

Differential Revision: D68843247

fbshipit-source-id: 730f3eba8c26959825cd9c3897f055a02a5f9591
  • Loading branch information
rubennorte authored and facebook-github-bot committed Jan 29, 2025
1 parent 17c3429 commit 2c406d8
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ ${Object.entries(definitions.common)
*/
export const ${flagName}: Getter<${typeof flagConfig.defaultValue}> = createNativeFlagGetter('${flagName}', ${JSON.stringify(
flagConfig.defaultValue,
)});`,
)}${flagConfig.skipNativeAPI === true ? ', true' : ''});`,
)
.join('\n')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<1292b8fff14cbf6ea3bffea28d0428bc>>
* @generated SignedSource<<3c609bd4ace6147f8f32af07a5e3cc05>>
* @flow strict
*/

Expand Down Expand Up @@ -169,11 +169,11 @@ export const commonTestFlag: Getter<boolean> = createNativeFlagGetter('commonTes
/**
* Common flag for testing (without native implementation). Do NOT modify.
*/
export const commonTestFlagWithoutNativeImplementation: Getter<boolean> = createNativeFlagGetter('commonTestFlagWithoutNativeImplementation', false);
export const commonTestFlagWithoutNativeImplementation: Getter<boolean> = createNativeFlagGetter('commonTestFlagWithoutNativeImplementation', false, true);
/**
* The bridgeless architecture enables the event loop by default. This feature flag allows us to force disabling it in specific instances.
*/
export const disableEventLoopOnBridgeless: Getter<boolean> = createNativeFlagGetter('disableEventLoopOnBridgeless', false);
export const disableEventLoopOnBridgeless: Getter<boolean> = createNativeFlagGetter('disableEventLoopOnBridgeless', false, true);
/**
* Prevent FabricMountingManager from reordering mountitems, which may lead to invalid state on the UI thread
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,13 @@ type NativeFeatureFlags = $NonMaybeType<typeof NativeReactNativeFeatureFlags>;
export function createNativeFlagGetter<K: $Keys<NativeFeatureFlags>>(
configName: K,
defaultValue: ReturnType<$NonMaybeType<NativeFeatureFlags[K]>>,
skipUnavailableNativeModuleError: boolean = false,
): Getter<ReturnType<$NonMaybeType<NativeFeatureFlags[K]>>> {
return createGetter(
configName,
() => {
const valueFromNative = NativeReactNativeFeatureFlags?.[configName]?.();
if (valueFromNative == null) {
if (valueFromNative == null && !skipUnavailableNativeModuleError) {
logUnavailableNativeModuleError(configName);
}
return valueFromNative;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ describe('ReactNativeFeatureFlags', () => {
);
});

it('should provide default values for common flags and NOT log an error if the native module is NOT available and `skipNativeAPI` is used', () => {
const ReactNativeFeatureFlags = require('../ReactNativeFeatureFlags');
expect(
ReactNativeFeatureFlags.commonTestFlagWithoutNativeImplementation(),
).toBe(false);

expect(console.error).toHaveBeenCalledTimes(0);
});

it('should provide default values for common flags and log an error if the method in the native module is NOT available', () => {
jest.doMock('../specs/NativeReactNativeFeatureFlags', () => ({
__esModule: true,
Expand Down

0 comments on commit 2c406d8

Please sign in to comment.