Make host context use null as empty and only error in dev (#25609)

Makes it slightly more blazing. No host config actually uses null as any useful and we use this as a placeholder value anyway. It's also better for perf since it doesn't let two different hidden classes pass around. It's also a magic place holder that triggers error if we do try to access anything from it.
facebook · Nov 1, 2022 · 8e69bc4 · 8e69bc4
1 parent 5f7ef8c
commit 8e69bc4
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 109 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberHostContext.new.js b/packages/react-reconciler/src/ReactFiberHostContext.new.js
@@ -14,33 +14,26 @@ import type {Container, HostContext} from './ReactFiberHostConfig';
 import {getChildHostContext, getRootHostContext} from './ReactFiberHostConfig';
 import {createCursor, push, pop} from './ReactFiberStack.new';
 
-declare class NoContextT {}
-const NO_CONTEXT: NoContextT = ({}: any);
-
-const contextStackCursor: StackCursor<HostContext | NoContextT> = createCursor(
-  NO_CONTEXT,
-);
-const contextFiberStackCursor: StackCursor<Fiber | NoContextT> = createCursor(
-  NO_CONTEXT,
+const contextStackCursor: StackCursor<HostContext | null> = createCursor(null);
+const contextFiberStackCursor: StackCursor<Fiber | null> = createCursor(null);
+const rootInstanceStackCursor: StackCursor<Container | null> = createCursor(
+  null,
 );
-const rootInstanceStackCursor: StackCursor<
-  Container | NoContextT,
-> = createCursor(NO_CONTEXT);
-
-function requiredContext<Value>(c: Value | NoContextT): Value {
-  if (c === NO_CONTEXT) {
-    throw new Error(
-      'Expected host context to exist. This error is likely caused by a bug ' +
-        'in React. Please file an issue.',
-    );
-  }
 
+function requiredContext<Value>(c: Value | null): Value {
+  if (__DEV__) {
+    if (c === null) {
+      console.error(
+        'Expected host context to exist. This error is likely caused by a bug ' +
+          'in React. Please file an issue.',
+      );
+    }
+  }
   return (c: any);
 }
 
 function getCurrentRootHostContainer(): null | Container {
-  const container = rootInstanceStackCursor.current;
-  return container === NO_CONTEXT ? null : ((container: any): Container);
+  return rootInstanceStackCursor.current;
 }
 
 function getRootHostContainer(): Container {
@@ -61,7 +54,7 @@ function pushHostContainer(fiber: Fiber, nextRootInstance: Container) {
   // we'd have a different number of entries on the stack depending on
   // whether getRootHostContext() throws somewhere in renderer code or not.
   // So we push an empty value first. This lets us safely unwind on errors.
-  push(contextStackCursor, NO_CONTEXT, fiber);
+  push(contextStackCursor, null, fiber);
   const nextRootContext = getRootHostContext(nextRootInstance);
   // Now that we know this function doesn't throw, replace it.
   pop(contextStackCursor, fiber);

diff --git a/packages/react-reconciler/src/ReactFiberHostContext.old.js b/packages/react-reconciler/src/ReactFiberHostContext.old.js
@@ -14,33 +14,26 @@ import type {Container, HostContext} from './ReactFiberHostConfig';
 import {getChildHostContext, getRootHostContext} from './ReactFiberHostConfig';
 import {createCursor, push, pop} from './ReactFiberStack.old';
 
-declare class NoContextT {}
-const NO_CONTEXT: NoContextT = ({}: any);
-
-const contextStackCursor: StackCursor<HostContext | NoContextT> = createCursor(
-  NO_CONTEXT,
-);
-const contextFiberStackCursor: StackCursor<Fiber | NoContextT> = createCursor(
-  NO_CONTEXT,
+const contextStackCursor: StackCursor<HostContext | null> = createCursor(null);
+const contextFiberStackCursor: StackCursor<Fiber | null> = createCursor(null);
+const rootInstanceStackCursor: StackCursor<Container | null> = createCursor(
+  null,
 );
-const rootInstanceStackCursor: StackCursor<
-  Container | NoContextT,
-> = createCursor(NO_CONTEXT);
-
-function requiredContext<Value>(c: Value | NoContextT): Value {
-  if (c === NO_CONTEXT) {
-    throw new Error(
-      'Expected host context to exist. This error is likely caused by a bug ' +
-        'in React. Please file an issue.',
-    );
-  }
 
+function requiredContext<Value>(c: Value | null): Value {
+  if (__DEV__) {
+    if (c === null) {
+      console.error(
+        'Expected host context to exist. This error is likely caused by a bug ' +
+          'in React. Please file an issue.',
+      );
+    }
+  }
   return (c: any);
 }
 
 function getCurrentRootHostContainer(): null | Container {
-  const container = rootInstanceStackCursor.current;
-  return container === NO_CONTEXT ? null : ((container: any): Container);
+  return rootInstanceStackCursor.current;
 }
 
 function getRootHostContainer(): Container {
@@ -61,7 +54,7 @@ function pushHostContainer(fiber: Fiber, nextRootInstance: Container) {
   // we'd have a different number of entries on the stack depending on
   // whether getRootHostContext() throws somewhere in renderer code or not.
   // So we push an empty value first. This lets us safely unwind on errors.
-  push(contextStackCursor, NO_CONTEXT, fiber);
+  push(contextStackCursor, null, fiber);
   const nextRootContext = getRootHostContext(nextRootInstance);
   // Now that we know this function doesn't throw, replace it.
   pop(contextStackCursor, fiber);

diff --git a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js
@@ -30,72 +30,10 @@ describe('ReactFiberHostContext', () => {
 
   global.IS_REACT_ACT_ENVIRONMENT = true;
 
-  // @gate __DEV__
-  it('works with null host context', async () => {
-    let creates = 0;
-    const Renderer = ReactFiberReconciler({
-      prepareForCommit: function() {
-        return null;
-      },
-      resetAfterCommit: function() {},
-      getRootHostContext: function() {
-        return null;
-      },
-      getChildHostContext: function() {
-        return null;
-      },
-      shouldSetTextContent: function() {
-        return false;
-      },
-      createInstance: function() {
-        creates++;
-      },
-      finalizeInitialChildren: function() {
-        return null;
-      },
-      appendInitialChild: function() {
-        return null;
-      },
-      now: function() {
-        return 0;
-      },
-      appendChildToContainer: function() {
-        return null;
-      },
-      clearContainer: function() {},
-      getCurrentEventPriority: function() {
-        return DefaultEventPriority;
-      },
-      prepareRendererToRender: function() {},
-      resetRendererAfterRender: function() {},
-      supportsMutation: true,
-      requestPostPaintCallback: function() {},
-    });
-
-    const container = Renderer.createContainer(
-      /* root: */ null,
-      ConcurrentRoot,
-      null,
-      false,
-      '',
-      null,
-    );
-    act(() => {
-      Renderer.updateContainer(
-        <a>
-          <b />
-        </a>,
-        container,
-        /* parentComponent: */ null,
-        /* callback: */ null,
-      );
-    });
-    expect(creates).toBe(2);
-  });
-
   // @gate __DEV__
   it('should send the context to prepareForCommit and resetAfterCommit', () => {
     const rootContext = {};
+    const childContext = {};
     const Renderer = ReactFiberReconciler({
       prepareForCommit: function(hostContext) {
         expect(hostContext).toBe(rootContext);
@@ -105,10 +43,10 @@ describe('ReactFiberHostContext', () => {
         expect(hostContext).toBe(rootContext);
       },
       getRootHostContext: function() {
-        return null;
+        return rootContext;
       },
       getChildHostContext: function() {
-        return null;
+        return childContext;
       },
       shouldSetTextContent: function() {
         return false;