From 465f3b2fa4b6a6d4d1312eda68beb37c0a015495 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 27 Aug 2026 01:15:38 -0700 Subject: [PATCH] fix(core): leave React's factory alone once the app declared its runtime Patching React.createElement is what crashes an app whose styling library owns the element factory. Measured on a release build of a nativewind app: the heap grows without bound until Hermes aborts during startup. Bisected against a build with only that patch removed - `memo` stays patched there and the app survives - so the factory patch is the whole of it. Two earlier attempts treated this as a matter of replacing the property more carefully: skip it when it is an accessor, and read it back through the same object the write lands on. Both are correct on their own terms and both are kept, but neither stops the crash. The problem is not how the property is replaced. It is that replacing it at all routes every createElement call in the process - React's internals included - through machinery the host wrote for its own calls. So the patch is now skipped once `jsxRuntimes` says where the app's elements actually come from. Under the automatic JSX transform such an app never calls React.createElement anyway, so nothing is instrumented that was not already: the declared runtime is what records the taps. An app that declares nothing still gets the old behaviour. Verified on a simulator release build: with the declared runtime instrumented and the factory left alone, the app starts, reports views, and records taps - `t_actions` rows of type `tap`, and `view_action_count` on the view they belong to. The pre-fix build is the negative control, and it aborts before any of that. --- .../DdRumUserInteractionTracking.test.tsx | 27 +++++++++++++ .../DdRumUserInteractionTracking.tsx | 40 ++++++++++++++----- 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/packages/core/src/__tests__/rum/instrumentation/DdRumUserInteractionTracking.test.tsx b/packages/core/src/__tests__/rum/instrumentation/DdRumUserInteractionTracking.test.tsx index 12472ff00..6359fd12b 100644 --- a/packages/core/src/__tests__/rum/instrumentation/DdRumUserInteractionTracking.test.tsx +++ b/packages/core/src/__tests__/rum/instrumentation/DdRumUserInteractionTracking.test.tsx @@ -543,6 +543,33 @@ describe('startTracking with injected jsx runtimes', () => { ); }); + it("M leave React's own factory alone W the app declared its runtime", async () => { + // A styling library that owns the element factory can route React.createElement + // through machinery of its own. Replacing it then feeds that machinery calls it was + // never written to receive - measured on a release build as the heap growing until + // Hermes aborted at startup. Once the app has told us where its JSX comes from, + // there is nothing to gain there and a crash to lose. + const before = React.createElement; + const runtime: Record = { + jsx: jest.fn(), + jsxs: jest.fn() + }; + + DdRumUserInteractionTracking.startTracking({}, [runtime]); + + expect(React.createElement).toBe(before); + // the declared runtime is still instrumented - that is what records the taps + expect(runtime.jsx).not.toBe(before); + }); + + it("M patch React's own factory W no runtime was declared", async () => { + const before = React.createElement; + + DdRumUserInteractionTracking.startTracking({}); + + expect(React.createElement).not.toBe(before); + }); + it('M restore the injected runtime W stopTracking is called', async () => { const jsx = jest.fn(); const jsxs = jest.fn(); diff --git a/packages/core/src/rum/instrumentation/interactionTracking/DdRumUserInteractionTracking.tsx b/packages/core/src/rum/instrumentation/interactionTracking/DdRumUserInteractionTracking.tsx index 8d3c4226f..71f86c1cd 100644 --- a/packages/core/src/rum/instrumentation/interactionTracking/DdRumUserInteractionTracking.tsx +++ b/packages/core/src/rum/instrumentation/interactionTracking/DdRumUserInteractionTracking.tsx @@ -243,17 +243,37 @@ export class DdRumUserInteractionTracking { options ); + // React's own factory is left alone once the app has told us which runtime it + // compiles to. Two reasons, and the second one is why this is not merely tidy. + // + // Under the automatic JSX transform such an app never calls React.createElement - + // its elements come from the runtime it declared - so patching it buys no action. + // + // And it is not free. A styling library that owns the element factory can route + // React.createElement through machinery of its own; replacing it then feeds that + // machinery calls it was never written to receive, including React's internal ones. + // Measured on a release build of a nativewind app: the heap grew without bound until + // Hermes aborted during startup. Skipping this patch there is what stops the crash - + // replacing the factory more carefully does not, which two earlier attempts at this + // established the hard way. + // + // `memo` stays patched either way: it only restores the original onPress for + // comparison, and was measured not to contribute to the crash. + const appDeclaredItsRuntime = jsxRuntimes.length > 0; + const originalCreateElement = reactModule['createElement']; - replaceProperty( - reactModule, - 'createElement', - (...args: Parameters): any => { - return this.patchCreateElementFunction( - originalCreateElement, - args - ); - } - ); + if (!appDeclaredItsRuntime) { + replaceProperty( + reactModule, + 'createElement', + (...args: Parameters): any => { + return this.patchCreateElementFunction( + originalCreateElement, + args + ); + } + ); + } const runtimes: JsxRuntimeModule[] = []; try {