Skip to content

Commit

Permalink
Use getter instead of constant to provide access to FabricUIManager b…
Browse files Browse the repository at this point in the history
…inding

Summary:
The `FabricUIManager` module was providing access to the global binding defined by Fabric, adding proper typing for it.

The problem about this module is that it only evaluated the global binding once (during module evaluation) and it cached the result. If this module happened to be loaded before the binding is defined, which can happen in apps using the legacy renderer and Fabric together, this modules caches an `undefined` value indefinitely.

This refactors the module as a getter function to make sure this lazy behavior is handled correctly in the callsites.

Changelog: [Internal]

Reviewed By: javache

Differential Revision: D44065186

fbshipit-source-id: 7c5cfe674605f03ecb8ca0dabc4c823e0013da6b
  • Loading branch information
rubennorte authored and facebook-github-bot committed Mar 15, 2023
1 parent 913732b commit 0875796
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 12 deletions.
3 changes: 2 additions & 1 deletion Libraries/LayoutAnimation/LayoutAnimation.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
LayoutAnimationType,
} from '../Renderer/shims/ReactNativeTypes';

import {getFabricUIManager} from '../ReactNative/FabricUIManager';
import ReactNativeFeatureFlags from '../ReactNative/ReactNativeFeatureFlags';
import Platform from '../Utilities/Platform';

Expand Down Expand Up @@ -77,7 +78,7 @@ function configureNext(

// In Fabric, LayoutAnimations are unconditionally enabled for Android, and
// conditionally enabled on iOS (pending fully shipping; this is a temporary state).
const FabricUIManager: FabricUIManagerSpec = global?.nativeFabricUIManager;
const FabricUIManager = getFabricUIManager();
if (FabricUIManager?.configureNextLayoutAnimation) {
global?.nativeFabricUIManager?.configureNextLayoutAnimation(
config,
Expand Down
9 changes: 6 additions & 3 deletions Libraries/ReactNative/FabricUIManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ export type Spec = {|
+findShadowNodeByTag_DEPRECATED: (reactTag: number) => ?Node,
|};

const FabricUIManager: ?Spec = global.nativeFabricUIManager;

module.exports = FabricUIManager;
// This is exposed as a getter because apps using the legacy renderer AND
// Fabric can define the binding lazily. If we evaluated the global and cached
// it in the module we might be caching an `undefined` value before it is set.
export function getFabricUIManager(): ?Spec {
return global.nativeFabricUIManager;
}
15 changes: 7 additions & 8 deletions Libraries/ReactNative/UIManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import type {RootTag} from '../Types/RootTagTypes';
import type {Spec as FabricUIManagerSpec} from './FabricUIManager';
import type {Spec} from './NativeUIManager';

import {getFabricUIManager} from './FabricUIManager';
import nullthrows from 'nullthrows';

export interface UIManagerJSInterface extends Spec {
+getViewManagerConfig: (viewManagerName: string) => Object;
+hasViewManagerConfig: (viewManagerName: string) => boolean;
Expand Down Expand Up @@ -57,8 +60,7 @@ const UIManager = {
) => void,
): void {
if (isFabricReactTag(reactTag)) {
const FabricUIManager: FabricUIManagerSpec =
global?.nativeFabricUIManager;
const FabricUIManager = nullthrows(getFabricUIManager());
const shadowNode =
FabricUIManager.findShadowNodeByTag_DEPRECATED(reactTag);
if (shadowNode) {
Expand All @@ -84,8 +86,7 @@ const UIManager = {
) => void,
): void {
if (isFabricReactTag(reactTag)) {
const FabricUIManager: FabricUIManagerSpec =
global?.nativeFabricUIManager;
const FabricUIManager = nullthrows(getFabricUIManager());
const shadowNode =
FabricUIManager.findShadowNodeByTag_DEPRECATED(reactTag);
if (shadowNode) {
Expand Down Expand Up @@ -113,8 +114,7 @@ const UIManager = {
) => void,
): void {
if (isFabricReactTag(reactTag)) {
const FabricUIManager: FabricUIManagerSpec =
global?.nativeFabricUIManager;
const FabricUIManager = nullthrows(getFabricUIManager());
const shadowNode =
FabricUIManager.findShadowNodeByTag_DEPRECATED(reactTag);
const ancestorShadowNode =
Expand Down Expand Up @@ -155,8 +155,7 @@ const UIManager = {
console.warn(
'RCTUIManager.measureLayoutRelativeToParent method is deprecated and it will not be implemented in newer versions of RN (Fabric) - T47686450',
);
const FabricUIManager: FabricUIManagerSpec =
global?.nativeFabricUIManager;
const FabricUIManager = nullthrows(getFabricUIManager());
const shadowNode =
FabricUIManager.findShadowNodeByTag_DEPRECATED(reactTag);
if (shadowNode) {
Expand Down

0 comments on commit 0875796

Please sign in to comment.