From 75a8331130da4d47e434474bfa10afffd2dbbf8d Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Wed, 29 Jan 2025 05:28:59 -0800 Subject: [PATCH 1/2] Delete RawProps assignment operators (#49030) Summary: Overwriting another RawProps object via `operator=` is rarely what we want, and these objects should be considered immutable once constructed. This will catch issues such as D68633985 Changelog: [General][Changed] Removed `RawProps::operator=` Reviewed By: sammy-SC Differential Revision: D68797484 --- .../ReactCommon/react/renderer/core/RawProps.cpp | 16 ---------------- .../ReactCommon/react/renderer/core/RawProps.h | 11 ++++++----- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawProps.cpp b/packages/react-native/ReactCommon/react/renderer/core/RawProps.cpp index efe6ae574e26..7ed03caffe94 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/RawProps.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/RawProps.cpp @@ -104,10 +104,6 @@ inline bool isYogaStyleProp(const std::string& prop) { } } // namespace -RawProps::RawProps() { - mode_ = Mode::Empty; -} - /* * Creates an object with given `runtime` and `value`. */ @@ -149,18 +145,6 @@ RawProps::RawProps(const RawProps& other) noexcept { ignoreYogaStyleProps_ = other.ignoreYogaStyleProps_; } -RawProps& RawProps::operator=(const RawProps& other) noexcept { - mode_ = other.mode_; - if (mode_ == Mode::JSI) { - runtime_ = other.runtime_; - value_ = jsi::Value(*runtime_, other.value_); - } else if (mode_ == Mode::Dynamic) { - dynamic_ = other.dynamic_; - } - ignoreYogaStyleProps_ = other.ignoreYogaStyleProps_; - return *this; -} - void RawProps::parse(const RawPropsParser& parser) noexcept { react_native_assert(parser_ == nullptr && "A parser was already assigned."); parser_ = &parser; diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawProps.h b/packages/react-native/ReactCommon/react/renderer/core/RawProps.h index 3e63bce5a744..efe63e927be4 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/RawProps.h +++ b/packages/react-native/ReactCommon/react/renderer/core/RawProps.h @@ -43,7 +43,7 @@ class RawProps final { /* * Creates empty RawProps objects. */ - RawProps(); + RawProps() : mode_(Mode::Empty) {} /* * Creates an object with given `runtime` and `value`. @@ -51,10 +51,10 @@ class RawProps final { RawProps(jsi::Runtime& runtime, const jsi::Value& value) noexcept; explicit RawProps(const RawProps& rawProps) noexcept; - RawProps& operator=(const RawProps& other) noexcept; - RawProps(RawProps&& other) noexcept = default; - RawProps& operator=(RawProps&& other) noexcept = default; + + RawProps& operator=(const RawProps& other) noexcept = delete; + RawProps& operator=(RawProps&& other) noexcept = delete; /* * Creates an object with given `folly::dynamic` object. @@ -112,8 +112,9 @@ class RawProps final { /* * Source artefacts: */ + // Mode - mutable Mode mode_; + Mode mode_; // Case 1: Source data is represented as `jsi::Object`. jsi::Runtime* runtime_{}; From 9f9d27709a069aa1cf3e97592c8d395e445d754a Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Wed, 29 Jan 2025 05:28:59 -0800 Subject: [PATCH 2/2] Error when tests finish with pending tasks (#49032) Summary: Add a native API to validate the RuntimeScheduler has no pending tasks, and automatically validate after every test that there's no pending tasks left to execute. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D68797481 --- packages/react-native-fantom/runtime/setup.js | 14 +++++++++----- .../src/__tests__/Fantom-itest.js | 11 +++++++++++ .../__snapshots__/public-api-test.js.snap | 1 + .../src/private/specs/modules/NativeFantom.js | 1 + .../__tests__/IntersectionObserver-itest.js | 16 +++++++++------- 5 files changed, 31 insertions(+), 12 deletions(-) diff --git a/packages/react-native-fantom/runtime/setup.js b/packages/react-native-fantom/runtime/setup.js index c19e4ee234da..266ce801cdc3 100644 --- a/packages/react-native-fantom/runtime/setup.js +++ b/packages/react-native-fantom/runtime/setup.js @@ -70,7 +70,7 @@ const rootContext: Context = { beforeAllHooks: [], beforeEachHooks: [], afterAllHooks: [], - afterEachHooks: [], + afterEachHooks: [validateEmptyMessageQueue], children: [], focused: false, skipped: false, @@ -93,8 +93,8 @@ const globalDescribe = (global.describe = ( beforeAllHooks: [], beforeEachHooks: [], children: [], - focused: focused, - skipped: skipped, + focused, + skipped, }; currentContext.children.push(childContext); currentContext = childContext; @@ -137,8 +137,8 @@ const globalIt = title, parentContext: currentContext, implementation, - focused: focused, - skipped: skipped, + focused, + skipped, }); }); @@ -388,6 +388,10 @@ function reportTestSuiteResult(testSuiteResult: TestSuiteResult): void { ); } +function validateEmptyMessageQueue(): void { + NativeFantom.validateEmptyMessageQueue(); +} + global.$$RunTests$$ = () => { reportTestSuiteResult({ testResults: runSuite(currentContext), diff --git a/packages/react-native-fantom/src/__tests__/Fantom-itest.js b/packages/react-native-fantom/src/__tests__/Fantom-itest.js index 46b0304d3ca5..d218fc4575ef 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 Fantom from '..'; import * as React from 'react'; import {ScrollView, Text, TextInput, View} from 'react-native'; +import NativeFantom from 'react-native/src/private/specs/modules/NativeFantom'; import ensureInstance from 'react-native/src/private/utilities/ensureInstance'; import ReactNativeElement from 'react-native/src/private/webapis/dom/nodes/ReactNativeElement'; @@ -132,6 +133,16 @@ describe('Fantom', () => { }); expect(threw).toBe(true); }); + + it('should error when any scheduled tasks remain after the test', () => { + Fantom.scheduleTask(() => {}); + expect(() => NativeFantom.validateEmptyMessageQueue()).toThrow( + 'Exception in HostFunction: MessageQueue is not empty', + ); + + // Flushing queue to avoid this test failing + Fantom.runWorkLoop(); + }); }); describe('createRoot', () => { diff --git a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap index 94e9857002fe..8f02e3c717cf 100644 --- a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap +++ b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap @@ -10482,6 +10482,7 @@ interface Spec extends TurboModule { getMountingManagerLogs: (surfaceId: number) => Array; flushMessageQueue: () => void; flushEventQueue: () => void; + validateEmptyMessageQueue: () => void; getRenderedOutput: (surfaceId: number, config: RenderFormatOptions) => string; reportTestSuiteResultsJSON: (results: string) => void; } diff --git a/packages/react-native/src/private/specs/modules/NativeFantom.js b/packages/react-native/src/private/specs/modules/NativeFantom.js index a7afa3a0dd37..c8626726645f 100644 --- a/packages/react-native/src/private/specs/modules/NativeFantom.js +++ b/packages/react-native/src/private/specs/modules/NativeFantom.js @@ -79,6 +79,7 @@ interface Spec extends TurboModule { getMountingManagerLogs: (surfaceId: number) => Array; flushMessageQueue: () => void; flushEventQueue: () => void; + validateEmptyMessageQueue: () => void; getRenderedOutput: (surfaceId: number, config: RenderFormatOptions) => string; reportTestSuiteResultsJSON: (results: string) => void; } 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 db7f2365a946..327594c1296d 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 @@ -1507,16 +1507,18 @@ describe('IntersectionObserver', () => { const node = ensureReactNativeElement(maybeNode); - observer1 = new IntersectionObserver(() => {}); - observer2 = new IntersectionObserver(() => {}); + Fantom.runTask(() => { + observer1 = new IntersectionObserver(() => {}); + observer2 = new IntersectionObserver(() => {}); - observer1.observe(node); - observer2.observe(node); + observer1.observe(node); + observer2.observe(node); - observer1.unobserve(node); + observer1.unobserve(node); - // The second call shouldn't log errors (that would make the test fail). - observer2.unobserve(node); + // The second call shouldn't log errors (that would make the test fail). + observer2.unobserve(node); + }); }); });