From d5c998b557bec28849ccac9f88bc43d9421e92eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Fri, 3 Jan 2025 05:59:17 -0800 Subject: [PATCH 1/4] Allow configuring viewport width, height and device pixel ratio (#48433) Summary: Changelog: [internal] Allows customizing the root dimensions. Reviewed By: christophpurrer Differential Revision: D67693031 --- .../src/__tests__/Fantom-itest.js | 57 +++++++++++++++++++ packages/react-native-fantom/src/index.js | 33 +++++++++-- .../src/private/specs/modules/NativeFantom.js | 7 ++- 3 files changed, 92 insertions(+), 5 deletions(-) diff --git a/packages/react-native-fantom/src/__tests__/Fantom-itest.js b/packages/react-native-fantom/src/__tests__/Fantom-itest.js index 250c31d4d828..752a6fe94ffe 100644 --- a/packages/react-native-fantom/src/__tests__/Fantom-itest.js +++ b/packages/react-native-fantom/src/__tests__/Fantom-itest.js @@ -7,13 +7,47 @@ * @flow strict-local * @format * @oncall react_native + * @fantom_flags enableAccessToHostTreeInFabric:true */ import 'react-native/Libraries/Core/InitializeCore'; +import type {Root} from '..'; + import {createRoot, runTask} from '..'; import * as React from 'react'; import {Text, View} from 'react-native'; +import ReactNativeElement from 'react-native/src/private/webapis/dom/nodes/ReactNativeElement'; + +function getActualViewportDimensions(root: Root): { + viewportWidth: number, + viewportHeight: number, +} { + let maybeNode; + + runTask(() => { + root.render( + { + maybeNode = node; + }} + />, + ); + }); + + if (!(maybeNode instanceof ReactNativeElement)) { + throw new Error( + `Expected instance of ReactNativeElement but got ${String(maybeNode)}`, + ); + } + + const rect = maybeNode.getBoundingClientRect(); + return { + viewportWidth: rect.width, + viewportHeight: rect.height, + }; +} describe('Fantom', () => { describe('runTask', () => { @@ -103,6 +137,29 @@ describe('Fantom', () => { }); }); + describe('createRoot', () => { + it('allows creating a root with specific dimensions', () => { + const rootWithDefaults = createRoot(); + + expect(getActualViewportDimensions(rootWithDefaults)).toEqual({ + viewportWidth: 1000, + viewportHeight: 1000, + }); + + const rootWithCustomWidthAndHeight = createRoot({ + viewportWidth: 200, + viewportHeight: 600, + }); + + expect(getActualViewportDimensions(rootWithCustomWidthAndHeight)).toEqual( + { + viewportWidth: 200, + viewportHeight: 600, + }, + ); + }); + }); + describe('getRenderedOutput', () => { describe('toJSX', () => { it('default config', () => { diff --git a/packages/react-native-fantom/src/index.js b/packages/react-native-fantom/src/index.js index 67795cc5c67f..92e0bf0558ff 100644 --- a/packages/react-native-fantom/src/index.js +++ b/packages/react-native-fantom/src/index.js @@ -24,18 +24,41 @@ const nativeRuntimeScheduler = global.nativeRuntimeScheduler; const schedulerPriorityImmediate = nativeRuntimeScheduler.unstable_ImmediatePriority; +export type RootConfig = { + viewportWidth?: number, + viewportHeight?: number, + devicePixelRatio?: number, +}; + +const DEFAULT_VIEWPORT_WIDTH = 1000; +const DEFAULT_VIEWPORT_HEIGHT = 1000; +const DEFAULT_DEVICE_PIXEL_RATIO = 1; + class Root { #surfaceId: number; + #viewportWidth: number; + #viewportHeight: number; + #devicePixelRatio: number; + #hasRendered: boolean = false; - constructor() { + constructor(config?: RootConfig) { this.#surfaceId = globalSurfaceIdCounter; + this.#viewportWidth = config?.viewportWidth ?? DEFAULT_VIEWPORT_WIDTH; + this.#viewportHeight = config?.viewportHeight ?? DEFAULT_VIEWPORT_HEIGHT; + this.#devicePixelRatio = + config?.devicePixelRatio ?? DEFAULT_DEVICE_PIXEL_RATIO; globalSurfaceIdCounter += 10; } render(element: MixedElement) { if (!this.#hasRendered) { - NativeFantom.startSurface(this.#surfaceId); + NativeFantom.startSurface( + this.#surfaceId, + this.#viewportWidth, + this.#viewportHeight, + this.#devicePixelRatio, + ); this.#hasRendered = true; } @@ -59,6 +82,8 @@ class Root { // TODO: add an API to check if all surfaces were deallocated when tests are finished. } +export type {Root}; + const DEFAULT_TASK_PRIORITY = schedulerPriorityImmediate; /** @@ -108,6 +133,6 @@ export function runWorkLoop(): void { // TODO: Add option to define surface props and pass it to startSurface // Surfacep rops: concurrentRoot, surfaceWidth, surfaceHeight, layoutDirection, pointScaleFactor. -export function createRoot(): Root { - return new Root(); +export function createRoot(rootConfig?: RootConfig): Root { + return new Root(rootConfig); } diff --git a/packages/react-native/src/private/specs/modules/NativeFantom.js b/packages/react-native/src/private/specs/modules/NativeFantom.js index 57e8f7fda028..1fa0c0d2fb7f 100644 --- a/packages/react-native/src/private/specs/modules/NativeFantom.js +++ b/packages/react-native/src/private/specs/modules/NativeFantom.js @@ -19,7 +19,12 @@ export type RenderFormatOptions = { }; interface Spec extends TurboModule { - startSurface: (surfaceId: number) => void; + startSurface: ( + surfaceId: number, + viewportWidth: number, + viewportHeight: number, + devicePixelRatio: number, + ) => void; stopSurface: (surfaceId: number) => void; getMountingManagerLogs: (surfaceId: number) => Array; flushMessageQueue: () => void; From 0eb56c2f40793618f23d3383ca519b1cfcf191ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Fri, 3 Jan 2025 05:59:17 -0800 Subject: [PATCH 2/4] Set more sensible defaults for viewport width and height (#48436) Summary: Changelog: [internal] Changing the defaults to use something that resembles a real device (in this case the iPhone 14 which is a very common device). Reviewed By: christophpurrer Differential Revision: D67759914 --- .../src/__tests__/Fantom-itest.js | 6 +-- packages/react-native-fantom/src/index.js | 7 +-- .../__tests__/ReactNativeElement-itest.js | 8 +-- .../__tests__/IntersectionObserver-itest.js | 50 +++++++++++++++---- 4 files changed, 51 insertions(+), 20 deletions(-) diff --git a/packages/react-native-fantom/src/__tests__/Fantom-itest.js b/packages/react-native-fantom/src/__tests__/Fantom-itest.js index 752a6fe94ffe..830ce5fa84d3 100644 --- a/packages/react-native-fantom/src/__tests__/Fantom-itest.js +++ b/packages/react-native-fantom/src/__tests__/Fantom-itest.js @@ -142,8 +142,8 @@ describe('Fantom', () => { const rootWithDefaults = createRoot(); expect(getActualViewportDimensions(rootWithDefaults)).toEqual({ - viewportWidth: 1000, - viewportHeight: 1000, + viewportWidth: 390, + viewportHeight: 844, }); const rootWithCustomWidthAndHeight = createRoot({ @@ -246,7 +246,7 @@ describe('Fantom', () => { layoutMetrics-frame="{x:0,y:0,width:100,height:100}" layoutMetrics-layoutDirection="LeftToRight" layoutMetrics-overflowInset="{top:0,right:-0,bottom:-0,left:0}" - layoutMetrics-pointScaleFactor="1" + layoutMetrics-pointScaleFactor="3" width="100.000000" />, ); diff --git a/packages/react-native-fantom/src/index.js b/packages/react-native-fantom/src/index.js index 92e0bf0558ff..cf78e5d9fda7 100644 --- a/packages/react-native-fantom/src/index.js +++ b/packages/react-native-fantom/src/index.js @@ -30,9 +30,10 @@ export type RootConfig = { devicePixelRatio?: number, }; -const DEFAULT_VIEWPORT_WIDTH = 1000; -const DEFAULT_VIEWPORT_HEIGHT = 1000; -const DEFAULT_DEVICE_PIXEL_RATIO = 1; +// Defaults use iPhone 14 values (very common device). +const DEFAULT_VIEWPORT_WIDTH = 390; +const DEFAULT_VIEWPORT_HEIGHT = 844; +const DEFAULT_DEVICE_PIXEL_RATIO = 3; class Root { #surfaceId: number; diff --git a/packages/react-native/src/private/webapis/dom/nodes/__tests__/ReactNativeElement-itest.js b/packages/react-native/src/private/webapis/dom/nodes/__tests__/ReactNativeElement-itest.js index e7d71390e426..15d901527d39 100644 --- a/packages/react-native/src/private/webapis/dom/nodes/__tests__/ReactNativeElement-itest.js +++ b/packages/react-native/src/private/webapis/dom/nodes/__tests__/ReactNativeElement-itest.js @@ -890,9 +890,9 @@ describe('ReactNativeElement', () => { const boundingClientRect = element.getBoundingClientRect(); expect(boundingClientRect).toBeInstanceOf(DOMRect); expect(boundingClientRect.x).toBe(5); - expect(boundingClientRect.y).toBe(10); - expect(boundingClientRect.width).toBe(50); - expect(boundingClientRect.height).toBe(101); + expect(boundingClientRect.y).toBeCloseTo(10.33); + expect(boundingClientRect.width).toBeCloseTo(50.33); + expect(boundingClientRect.height).toBeCloseTo(100.33); Fantom.runTask(() => { root.render(); @@ -1131,7 +1131,7 @@ describe('ReactNativeElement', () => { const element = ensureReactNativeElement(lastElement); expect(element.offsetWidth).toBe(50); - expect(element.offsetHeight).toBe(101); + expect(element.offsetHeight).toBe(100); Fantom.runTask(() => { root.render(); diff --git a/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js b/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js index 8ad459de8af1..00f0938d795d 100644 --- a/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js +++ b/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js @@ -340,7 +340,10 @@ describe('IntersectionObserver', () => { let maybeNode; let observer: IntersectionObserver; - const root = Fantom.createRoot(); + const root = Fantom.createRoot({ + viewportWidth: 1000, + viewportHeight: 1000, + }); Fantom.runTask(() => { root.render( { let maybeNode; let observer: IntersectionObserver; - const root = Fantom.createRoot(); + const root = Fantom.createRoot({ + viewportWidth: 1000, + viewportHeight: 1000, + }); Fantom.runTask(() => { root.render( @@ -538,7 +544,10 @@ describe('IntersectionObserver', () => { let maybeNode; let observer: IntersectionObserver; - const root = Fantom.createRoot(); + const root = Fantom.createRoot({ + viewportWidth: 1000, + viewportHeight: 1000, + }); Fantom.runTask(() => { root.render( @@ -598,7 +607,10 @@ describe('IntersectionObserver', () => { let maybeNode; let observer: IntersectionObserver; - const root = Fantom.createRoot(); + const root = Fantom.createRoot({ + viewportWidth: 1000, + viewportHeight: 1000, + }); Fantom.runTask(() => { root.render( @@ -658,7 +670,10 @@ describe('IntersectionObserver', () => { let maybeNode; let observer: IntersectionObserver; - const root = Fantom.createRoot(); + const root = Fantom.createRoot({ + viewportWidth: 1000, + viewportHeight: 1000, + }); Fantom.runTask(() => { root.render( { let observer1: IntersectionObserver; let observer2: IntersectionObserver; - const root = Fantom.createRoot(); + const root = Fantom.createRoot({ + viewportWidth: 1000, + viewportHeight: 1000, + }); Fantom.runTask(() => { root.render( @@ -869,7 +887,10 @@ describe('IntersectionObserver', () => { let maybeNode; let observer: IntersectionObserver; - const root = Fantom.createRoot(); + const root = Fantom.createRoot({ + viewportWidth: 1000, + viewportHeight: 1000, + }); Fantom.runTask(() => { root.render( { let maybeNode; let observer: IntersectionObserver; - const root = Fantom.createRoot(); + const root = Fantom.createRoot({ + viewportWidth: 1000, + viewportHeight: 1000, + }); Fantom.runTask(() => { root.render( @@ -989,7 +1013,10 @@ describe('IntersectionObserver', () => { let maybeNode; let observer: IntersectionObserver; - const root = Fantom.createRoot(); + const root = Fantom.createRoot({ + viewportWidth: 1000, + viewportHeight: 1000, + }); Fantom.runTask(() => { root.render( @@ -1049,7 +1076,10 @@ describe('IntersectionObserver', () => { let maybeNode; let observer: IntersectionObserver; - const root = Fantom.createRoot(); + const root = Fantom.createRoot({ + viewportWidth: 1000, + viewportHeight: 1000, + }); Fantom.runTask(() => { root.render( Date: Fri, 3 Jan 2025 05:59:17 -0800 Subject: [PATCH 3/4] Small refactor to lazy load RendererProxy (#48437) Summary: Changelog: [internal] Just a small refactor to slightly improve readability (and potentially performance). Reviewed By: christophpurrer Differential Revision: D67520444 --- .../private/webapis/dom/nodes/ReadOnlyNode.js | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/react-native/src/private/webapis/dom/nodes/ReadOnlyNode.js b/packages/react-native/src/private/webapis/dom/nodes/ReadOnlyNode.js index 39bb71ad00ce..abd9d090c459 100644 --- a/packages/react-native/src/private/webapis/dom/nodes/ReadOnlyNode.js +++ b/packages/react-native/src/private/webapis/dom/nodes/ReadOnlyNode.js @@ -303,11 +303,18 @@ export function setInstanceHandle( node[INSTANCE_HANDLE_KEY] = instanceHandle; } +let RendererProxy; +function getRendererProxy() { + if (RendererProxy == null) { + // Lazy import Fabric here to avoid DOM Node APIs classes from having side-effects. + // With a static import we can't use these classes for Paper-only variants. + RendererProxy = require('../../../../../Libraries/ReactNative/RendererProxy'); + } + return RendererProxy; +} + export function getShadowNode(node: ReadOnlyNode): ?ShadowNode { - // Lazy import Fabric here to avoid DOM Node APIs classes from having side-effects. - // With a static import we can't use these classes for Paper-only variants. - const RendererProxy = require('../../../../../Libraries/ReactNative/RendererProxy'); - return RendererProxy.getNodeFromInternalInstanceHandle( + return getRendererProxy().getNodeFromInternalInstanceHandle( getInstanceHandle(node), ); } @@ -351,11 +358,10 @@ function getNodeSiblingsAndPosition( export function getPublicInstanceFromInternalInstanceHandle( instanceHandle: InternalInstanceHandle, ): ?ReadOnlyNode { - // Lazy import Fabric here to avoid DOM Node APIs classes from having side-effects. - // With a static import we can't use these classes for Paper-only variants. - const RendererProxy = require('../../../../../Libraries/ReactNative/RendererProxy'); const mixedPublicInstance = - RendererProxy.getPublicInstanceFromInternalInstanceHandle(instanceHandle); + getRendererProxy().getPublicInstanceFromInternalInstanceHandle( + instanceHandle, + ); // $FlowExpectedError[incompatible-return] React defines public instances as "mixed" because it can't access the definition from React Native. return mixedPublicInstance; } From 320f1be8bd524e36a8ca00bc942909add4637bba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Fri, 3 Jan 2025 05:59:17 -0800 Subject: [PATCH 4/4] Extract ensureInstance utility from tests (#48434) Summary: Changelog: [internal] We were starting to repeat this logic too much in tests, so extracting so we can reuse it. Reviewed By: rshest Differential Revision: D67520443 --- .../src/__tests__/Fantom-itest.js | 9 +++---- .../src/private/utilities/ensureInstance.js | 21 ++++++++++++++++ .../__tests__/ReactNativeElement-itest.js | 9 ++----- .../dom/nodes/__tests__/ReadOnlyText-itest.js | 25 +++---------------- .../__tests__/IntersectionObserver-itest.js | 13 +--------- .../__tests__/MutationObserver-itest.js | 13 +--------- 6 files changed, 32 insertions(+), 58 deletions(-) create mode 100644 packages/react-native/src/private/utilities/ensureInstance.js diff --git a/packages/react-native-fantom/src/__tests__/Fantom-itest.js b/packages/react-native-fantom/src/__tests__/Fantom-itest.js index 830ce5fa84d3..773f92665b9c 100644 --- a/packages/react-native-fantom/src/__tests__/Fantom-itest.js +++ b/packages/react-native-fantom/src/__tests__/Fantom-itest.js @@ -17,6 +17,7 @@ import type {Root} from '..'; import {createRoot, runTask} from '..'; import * as React from 'react'; import {Text, View} from 'react-native'; +import ensureInstance from 'react-native/src/private/utilities/ensureInstance'; import ReactNativeElement from 'react-native/src/private/webapis/dom/nodes/ReactNativeElement'; function getActualViewportDimensions(root: Root): { @@ -36,13 +37,9 @@ function getActualViewportDimensions(root: Root): { ); }); - if (!(maybeNode instanceof ReactNativeElement)) { - throw new Error( - `Expected instance of ReactNativeElement but got ${String(maybeNode)}`, - ); - } + const node = ensureInstance(maybeNode, ReactNativeElement); - const rect = maybeNode.getBoundingClientRect(); + const rect = node.getBoundingClientRect(); return { viewportWidth: rect.width, viewportHeight: rect.height, diff --git a/packages/react-native/src/private/utilities/ensureInstance.js b/packages/react-native/src/private/utilities/ensureInstance.js new file mode 100644 index 000000000000..31a0e4124dd5 --- /dev/null +++ b/packages/react-native/src/private/utilities/ensureInstance.js @@ -0,0 +1,21 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @flow strict + */ + +export default function ensureInstance(value: mixed, Class: Class): T { + if (!(value instanceof Class)) { + // $FlowIssue[incompatible-use] + const className = Class.name; + throw new Error( + `Expected instance of ${className} but got ${String(value)}`, + ); + } + + return value; +} diff --git a/packages/react-native/src/private/webapis/dom/nodes/__tests__/ReactNativeElement-itest.js b/packages/react-native/src/private/webapis/dom/nodes/__tests__/ReactNativeElement-itest.js index 15d901527d39..7484d5577ef5 100644 --- a/packages/react-native/src/private/webapis/dom/nodes/__tests__/ReactNativeElement-itest.js +++ b/packages/react-native/src/private/webapis/dom/nodes/__tests__/ReactNativeElement-itest.js @@ -18,6 +18,7 @@ import { NativeText, NativeVirtualText, } from '../../../../../../Libraries/Text/TextNativeComponent'; +import ensureInstance from '../../../../utilities/ensureInstance'; import HTMLCollection from '../../oldstylecollections/HTMLCollection'; import NodeList from '../../oldstylecollections/NodeList'; import ReactNativeElement from '../ReactNativeElement'; @@ -26,13 +27,7 @@ import * as Fantom from '@react-native/fantom'; import * as React from 'react'; function ensureReactNativeElement(value: mixed): ReactNativeElement { - if (!(value instanceof ReactNativeElement)) { - throw new Error( - `Expected instance of ReactNativeElement but got ${String(value)}`, - ); - } - - return value; + return ensureInstance(value, ReactNativeElement); } /* eslint-disable no-bitwise */ diff --git a/packages/react-native/src/private/webapis/dom/nodes/__tests__/ReadOnlyText-itest.js b/packages/react-native/src/private/webapis/dom/nodes/__tests__/ReadOnlyText-itest.js index 74987698647b..808f2f53e9ed 100644 --- a/packages/react-native/src/private/webapis/dom/nodes/__tests__/ReadOnlyText-itest.js +++ b/packages/react-native/src/private/webapis/dom/nodes/__tests__/ReadOnlyText-itest.js @@ -13,6 +13,7 @@ import '../../../../../../Libraries/Core/InitializeCore.js'; import {NativeText} from '../../../../../../Libraries/Text/TextNativeComponent'; +import ensureInstance from '../../../../utilities/ensureInstance'; import ReactNativeElement from '../ReactNativeElement'; import ReadOnlyNode from '../ReadOnlyNode'; import ReadOnlyText from '../ReadOnlyText'; @@ -21,33 +22,15 @@ import invariant from 'invariant'; import * as React from 'react'; function ensureReadOnlyText(value: mixed): ReadOnlyText { - if (!(value instanceof ReadOnlyText)) { - throw new Error( - `Expected instance of ReactOnlyNode but got ${String(value)}`, - ); - } - - return value; + return ensureInstance(value, ReadOnlyText); } function ensureReadOnlyNode(value: mixed): ReadOnlyNode { - if (!(value instanceof ReadOnlyNode)) { - throw new Error( - `Expected instance of ReactOnlyNode but got ${String(value)}`, - ); - } - - return value; + return ensureInstance(value, ReadOnlyNode); } function ensureReactNativeElement(value: mixed): ReactNativeElement { - if (!(value instanceof ReactNativeElement)) { - throw new Error( - `Expected instance of ReactNativeElement but got ${String(value)}`, - ); - } - - return value; + return ensureInstance(value, ReactNativeElement); } describe('ReadOnlyText', () => { diff --git a/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js b/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js index 00f0938d795d..aebb8226a1b4 100644 --- a/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js +++ b/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js @@ -24,23 +24,12 @@ import '../../../../../Libraries/Core/InitializeCore.js'; import ScrollView from '../../../../../Libraries/Components/ScrollView/ScrollView'; import View from '../../../../../Libraries/Components/View/View'; +import ensureInstance from '../../../utilities/ensureInstance'; declare const IntersectionObserver: Class; setUpIntersectionObserver(); -function ensureInstance(value: mixed, Class: Class): T { - if (!(value instanceof Class)) { - // $FlowExpectedError[incompatible-use] - const className = Class.name; - throw new Error( - `Expected instance of ${className} but got ${String(value)}`, - ); - } - - return value; -} - function ensureReactNativeElement(value: mixed): ReactNativeElement { return ensureInstance(value, ReactNativeElement); } diff --git a/packages/react-native/src/private/webapis/mutationobserver/__tests__/MutationObserver-itest.js b/packages/react-native/src/private/webapis/mutationobserver/__tests__/MutationObserver-itest.js index 8163f88a15a5..788107067f5a 100644 --- a/packages/react-native/src/private/webapis/mutationobserver/__tests__/MutationObserver-itest.js +++ b/packages/react-native/src/private/webapis/mutationobserver/__tests__/MutationObserver-itest.js @@ -20,23 +20,12 @@ import nullthrows from 'nullthrows'; import * as React from 'react'; import '../../../../../Libraries/Core/InitializeCore.js'; +import ensureInstance from '../../../utilities/ensureInstance'; declare const MutationObserver: Class; setUpMutationObserver(); -function ensureInstance(value: mixed, Class: Class): T { - if (!(value instanceof Class)) { - // $FlowExpectedError[incompatible-use] - const className = Class.name; - throw new Error( - `Expected instance of ${className} but got ${String(value)}`, - ); - } - - return value; -} - function ensureReactNativeElement(value: mixed): ReactNativeElement { return ensureInstance(value, ReactNativeElement); }