fix: safely handle internal props (#15)

pmndrs · Sep 6, 2022 · 39e0815 · 39e0815
1 parent 8800498
commit 39e0815
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 35 deletions.
diff --git a/src/index.test.tsx b/src/index.test.tsx
@@ -13,10 +13,6 @@ global.IS_REACT_ACT_ENVIRONMENT = true
 // Mock scheduler to test React features
 vi.mock('scheduler', () => require('scheduler/unstable_mock'))
 
-// Silence react-dom & react-dom/client mismatch
-const logError = global.console.error.bind(global.console.error)
-global.console.error = (...args: any[]) => !args[0].startsWith('Warning') && logError(...args)
-
 interface ReactProps<T> {
   key?: React.Key
   ref?: React.Ref<T>
@@ -47,35 +43,6 @@ it('should go through lifecycle', async () => {
   expect(lifecycle).toStrictEqual(['render', 'useInsertionEffect', 'ref', 'useLayoutEffect', 'useEffect'])
 })
 
-it('should pass tree as JSON from render', async () => {
-  let container!: HostContainer
-  await act(async () => {
-    container = render(
-      <element parent>
-        <element child>text</element>
-      </element>,
-    )
-  })
-
-  expect(container.head).toStrictEqual({
-    type: 'element',
-    props: { parent: true },
-    children: [
-      {
-        type: 'element',
-        props: { child: true },
-        children: [
-          {
-            type: 'text',
-            props: { value: 'text' },
-            children: [],
-          },
-        ],
-      },
-    ],
-  })
-})
-
 it('should render no-op elements', async () => {
   let container!: HostContainer
 
@@ -119,6 +86,24 @@ it('should render native elements', async () => {
   await act(async () => (container = render(<element foo />)))
   expect(container.head).toStrictEqual({ type: 'element', props: { foo: true }, children: [] })
 
+  // Child mount
+  await act(async () => {
+    container = render(
+      <element foo>
+        <element />
+      </element>,
+    )
+  })
+  expect(container.head).toStrictEqual({
+    type: 'element',
+    props: { foo: true },
+    children: [{ type: 'element', props: {}, children: [] }],
+  })
+
+  // Child unmount
+  await act(async () => (container = render(<element foo />)))
+  expect(container.head).toStrictEqual({ type: 'element', props: { foo: true }, children: [] })
+
   // Unmount
   await act(async () => (container = render(<></>)))
   expect(container.head).toBe(null)

diff --git a/src/index.ts b/src/index.ts
@@ -28,6 +28,18 @@ interface HostConfig {
   noTimeout: -1
 }
 
+// react-reconciler exposes some sensitive props. We don't want them exposed in public instances
+const REACT_INTERNAL_PROPS = ['ref', 'key', 'children']
+function getInstanceProps(props: Reconciler.Fiber['pendingProps']): HostConfig['props'] {
+  const instanceProps: HostConfig['props'] = {}
+
+  for (const key in props) {
+    if (!REACT_INTERNAL_PROPS.includes(key)) instanceProps[key] = props[key]
+  }
+
+  return instanceProps
+}
+
 const reconciler = Reconciler<
   HostConfig['type'],
   HostConfig['props'],
@@ -51,7 +63,7 @@ const reconciler = Reconciler<
   scheduleTimeout: setTimeout,
   cancelTimeout: clearTimeout,
   noTimeout: -1,
-  createInstance: (type, { ref, key, children, ...props }) => ({ type, props, children: [] }),
+  createInstance: (type, props) => ({ type, props: getInstanceProps(props), children: [] }),
   hideInstance() {},
   unhideInstance() {},
   createTextInstance: (value) => ({ type: 'text', props: { value }, children: [] }),
@@ -69,7 +81,7 @@ const reconciler = Reconciler<
   shouldSetTextContent: () => false,
   finalizeInitialChildren: () => false,
   prepareUpdate: () => ({}),
-  commitUpdate: (instance, _, __, ___, { ref, key, children, ...props }) => (instance.props = props),
+  commitUpdate: (instance, _, __, ___, props) => (instance.props = getInstanceProps(props)),
   commitTextUpdate: (instance, _, value) => (instance.props.value = value),
   prepareForCommit: () => null,
   resetAfterCommit() {},