From 3e3589f69bf8ed6124ad1831e011b2fccddde2be Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 27 Mar 2024 13:29:30 -0400 Subject: [PATCH] Make ART Concurrent if Legacy Mode is disabled Updated flowing in from above as well as setStates will now be batched using "discrete" priority but should still happen same task. We need to use the right scheduler in tests for act to work properly. --- 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 9b065a2b1185c..bf044d34b0441 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 67b98d9d3e776..15c01e8e89721 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 () => {