From 5fcaa0a832db9573364cb73738e0a3b4cf2d27f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 2 Apr 2024 15:00:30 -0400 Subject: [PATCH] Make ART Concurrent if Legacy Mode is disabled (#28662) Pulling this out of #28657. This runs react-art in concurrent mode if disableLegacyMode is true. Effectively this means that the OSS version will be in concurrent mode and the `.modern.js` version for Meta will be in concurrent mode, once the flag flips for modern, but the `.classic.js` version for Meta will be in legacy mode. Updates flowing in from above flush synchronously so that they commit as a unit. This also ensures that refs are resolved before the parent life cycles. setStates deep in the tree will now be batched using "discrete" priority but should still happen same task. --- packages/react-art/src/ReactART.js | 24 ++++++++++--- .../react-art/src/__tests__/ReactART-test.js | 34 ++++++++++++------- 2 files changed, 41 insertions(+), 17 deletions(-) diff --git a/packages/react-art/src/ReactART.js b/packages/react-art/src/ReactART.js index 9b065a2b1185..bf044d34b044 100644 --- a/packages/react-art/src/ReactART.js +++ b/packages/react-art/src/ReactART.js @@ -7,15 +7,17 @@ import * as React from 'react'; import ReactVersion from 'shared/ReactVersion'; -import {LegacyRoot} from 'react-reconciler/src/ReactRootTags'; +import {LegacyRoot, ConcurrentRoot} from 'react-reconciler/src/ReactRootTags'; import { createContainer, updateContainer, injectIntoDevTools, + flushSync, } from 'react-reconciler/src/ReactFiberReconciler'; import Transform from 'art/core/transform'; import Mode from 'art/modes/current'; import FastNoSideEffects from 'art/modes/fast-noSideEffects'; +import {disableLegacyMode} from 'shared/ReactFeatureFlags'; import {TYPES, childrenAsString} from './ReactARTInternals'; @@ -68,13 +70,17 @@ class Surface extends React.Component { this._mountNode = createContainer( this._surface, - LegacyRoot, + disableLegacyMode ? ConcurrentRoot : LegacyRoot, null, false, false, '', ); - updateContainer(this.props.children, this._mountNode, this); + // We synchronously flush updates coming from above so that they commit together + // and so that refs resolve before the parent life cycles. + flushSync(() => { + updateContainer(this.props.children, this._mountNode, this); + }); } componentDidUpdate(prevProps, prevState) { @@ -84,7 +90,11 @@ class Surface extends React.Component { this._surface.resize(+props.width, +props.height); } - updateContainer(this.props.children, this._mountNode, this); + // We synchronously flush updates coming from above so that they commit together + // and so that refs resolve before the parent life cycles. + flushSync(() => { + updateContainer(this.props.children, this._mountNode, this); + }); if (this._surface.render) { this._surface.render(); @@ -92,7 +102,11 @@ class Surface extends React.Component { } componentWillUnmount() { - updateContainer(null, this._mountNode, this); + // We synchronously flush updates coming from above so that they commit together + // and so that refs resolve before the parent life cycles. + flushSync(() => { + updateContainer(null, this._mountNode, this); + }); } render() { diff --git a/packages/react-art/src/__tests__/ReactART-test.js b/packages/react-art/src/__tests__/ReactART-test.js index 67b98d9d3e77..15c01e8e8972 100644 --- a/packages/react-art/src/__tests__/ReactART-test.js +++ b/packages/react-art/src/__tests__/ReactART-test.js @@ -11,7 +11,8 @@ 'use strict'; -import * as React from 'react'; +const React = require('react'); +const Scheduler = require('scheduler'); import * as ReactART from 'react-art'; import ARTSVGMode from 'art/modes/svg'; @@ -22,22 +23,28 @@ import Circle from 'react-art/Circle'; import Rectangle from 'react-art/Rectangle'; import Wedge from 'react-art/Wedge'; +const {act, waitFor} = require('internal-test-utils'); + // Isolate DOM renderer. jest.resetModules(); +// share isomorphic +jest.mock('scheduler', () => Scheduler); +jest.mock('react', () => React); +const ReactDOM = require('react-dom'); const ReactDOMClient = require('react-dom/client'); -let act = require('internal-test-utils').act; // Isolate the noop renderer jest.resetModules(); +// share isomorphic +jest.mock('scheduler', () => Scheduler); +jest.mock('react', () => React); const ReactNoop = require('react-noop-renderer'); -const Scheduler = require('scheduler'); let Group; let Shape; let Surface; let TestComponent; -let waitFor; let groupRef; const Missing = {}; @@ -68,6 +75,11 @@ describe('ReactART', () => { let container; beforeEach(() => { + jest.resetModules(); + // share isomorphic + jest.mock('scheduler', () => Scheduler); + jest.mock('react', () => React); + container = document.createElement('div'); document.body.appendChild(container); @@ -77,8 +89,6 @@ describe('ReactART', () => { Shape = ReactART.Shape; Surface = ReactART.Surface; - ({waitFor} = require('internal-test-utils')); - groupRef = React.createRef(); TestComponent = class extends React.Component { group = groupRef; @@ -409,8 +419,6 @@ describe('ReactART', () => { ); } - // Using test renderer instead of the DOM renderer here because async - // testing APIs for the DOM renderer don't exist. ReactNoop.render( @@ -423,7 +431,9 @@ describe('ReactART', () => { await waitFor(['A']); const root = ReactDOMClient.createRoot(container); - await act(() => { + // We use flush sync here because we expect this to render in between + // while the concurrent render is yieldy where as act would flush both. + ReactDOM.flushSync(() => { root.render( @@ -434,8 +444,6 @@ describe('ReactART', () => { ); }); - expect(ops).toEqual([null, 'ART']); - ops = []; await waitFor(['B', 'C']); @@ -447,9 +455,11 @@ describe('ReactARTComponents', () => { let ReactTestRenderer; beforeEach(() => { jest.resetModules(); + // share isomorphic + jest.mock('scheduler', () => Scheduler); + jest.mock('react', () => React); // Isolate test renderer. ReactTestRenderer = require('react-test-renderer'); - act = require('internal-test-utils').act; }); it('should generate a with props for drawing the Circle', async () => {