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.
Sebastian Markbåge committed
Apr 2, 2024 at 15:00 UTC
5fcaa0a832db9573364cb73738e0a3b4cf2d27f2
2 files changed
+41
-17
packages/react-art/src/ReactART.js
+19
-5
@@ -7,15 +7,17 @@
7
8
import * as React from 'react';
9
import ReactVersion from 'shared/ReactVersion';
10
-import {LegacyRoot} from 'react-reconciler/src/ReactRootTags';
10
+import {LegacyRoot, ConcurrentRoot} from 'react-reconciler/src/ReactRootTags';
11
import {
12
createContainer,
13
updateContainer,
14
injectIntoDevTools,
15
+ flushSync,
16
} from 'react-reconciler/src/ReactFiberReconciler';
17
import Transform from 'art/core/transform';
18
import Mode from 'art/modes/current';
19
import FastNoSideEffects from 'art/modes/fast-noSideEffects';
20
+import {disableLegacyMode} from 'shared/ReactFeatureFlags';
21
22
import {TYPES, childrenAsString} from './ReactARTInternals';
23
@@ -68,13 +70,17 @@ class Surface extends React.Component {
70
71
this._mountNode = createContainer(
72
this._surface,
71
- LegacyRoot,
73
+ disableLegacyMode ? ConcurrentRoot : LegacyRoot,
74
null,
75
false,
76
false,
77
'',
78
);
77
- updateContainer(this.props.children, this._mountNode, this);
79
+ // We synchronously flush updates coming from above so that they commit together
80
+ // and so that refs resolve before the parent life cycles.
81
+ flushSync(() => {
82
+ updateContainer(this.props.children, this._mountNode, this);
83
+ });
84
}
85
86
componentDidUpdate(prevProps, prevState) {
@@ -84,7 +90,11 @@ class Surface extends React.Component {
90
this._surface.resize(+props.width, +props.height);
91
}
92
87
- updateContainer(this.props.children, this._mountNode, this);
93
+ // We synchronously flush updates coming from above so that they commit together
94
+ // and so that refs resolve before the parent life cycles.
95
+ flushSync(() => {
96
+ updateContainer(this.props.children, this._mountNode, this);
97
+ });
98
99
if (this._surface.render) {
100
this._surface.render();
@@ -92,7 +102,11 @@ class Surface extends React.Component {
102
}
103
104
componentWillUnmount() {
95
- updateContainer(null, this._mountNode, this);
105
+ // We synchronously flush updates coming from above so that they commit together
106
+ // and so that refs resolve before the parent life cycles.
107
+ flushSync(() => {
108
+ updateContainer(null, this._mountNode, this);
109
+ });
110
}
111
112
render() {
packages/react-art/src/__tests__/ReactART-test.js
+22
-12
@@ -11,7 +11,8 @@
11
12
'use strict';
13
14
-import * as React from 'react';
14
+const React = require('react');
15
+const Scheduler = require('scheduler');
16
17
import * as ReactART from 'react-art';
18
import ARTSVGMode from 'art/modes/svg';
@@ -22,22 +23,28 @@ import Circle from 'react-art/Circle';
23
import Rectangle from 'react-art/Rectangle';
24
import Wedge from 'react-art/Wedge';
25
26
+const {act, waitFor} = require('internal-test-utils');
27
+
28
// Isolate DOM renderer.
29
jest.resetModules();
30
+// share isomorphic
31
+jest.mock('scheduler', () => Scheduler);
32
+jest.mock('react', () => React);
33
+const ReactDOM = require('react-dom');
34
const ReactDOMClient = require('react-dom/client');
28
-let act = require('internal-test-utils').act;
35
36
// Isolate the noop renderer
37
jest.resetModules();
38
+// share isomorphic
39
+jest.mock('scheduler', () => Scheduler);
40
+jest.mock('react', () => React);
41
const ReactNoop = require('react-noop-renderer');
33
-const Scheduler = require('scheduler');
42
43
let Group;
44
let Shape;
45
let Surface;
46
let TestComponent;
47
40
-let waitFor;
48
let groupRef;
49
50
const Missing = {};
@@ -68,6 +75,11 @@ describe('ReactART', () => {
75
let container;
76
77
beforeEach(() => {
78
+ jest.resetModules();
79
+ // share isomorphic
80
+ jest.mock('scheduler', () => Scheduler);
81
+ jest.mock('react', () => React);
82
+
83
container = document.createElement('div');
84
document.body.appendChild(container);
85
@@ -77,8 +89,6 @@ describe('ReactART', () => {
89
Shape = ReactART.Shape;
90
Surface = ReactART.Surface;
91
80
- ({waitFor} = require('internal-test-utils'));
81
-
92
groupRef = React.createRef();
93
TestComponent = class extends React.Component {
94
group = groupRef;
@@ -409,8 +419,6 @@ describe('ReactART', () => {
419
);
420
}
421
412
- // Using test renderer instead of the DOM renderer here because async
413
- // testing APIs for the DOM renderer don't exist.
422
ReactNoop.render(
423
<CurrentRendererContext.Provider value="Test">
424
<Yield value="A" />
@@ -423,7 +431,9 @@ describe('ReactART', () => {
431
await waitFor(['A']);
432
433
const root = ReactDOMClient.createRoot(container);
426
- await act(() => {
434
+ // We use flush sync here because we expect this to render in between
435
+ // while the concurrent render is yieldy where as act would flush both.
436
+ ReactDOM.flushSync(() => {
437
root.render(
438
<Surface>
439
<LogCurrentRenderer />
@@ -434,8 +444,6 @@ describe('ReactART', () => {
444
);
445
});
446
437
- expect(ops).toEqual([null, 'ART']);
438
-
447
ops = [];
448
await waitFor(['B', 'C']);
449
@@ -447,9 +455,11 @@ describe('ReactARTComponents', () => {
455
let ReactTestRenderer;
456
beforeEach(() => {
457
jest.resetModules();
458
+ // share isomorphic
459
+ jest.mock('scheduler', () => Scheduler);
460
+ jest.mock('react', () => React);
461
// Isolate test renderer.
462
ReactTestRenderer = require('react-test-renderer');
452
- act = require('internal-test-utils').act;
463
});
464
465
it('should generate a <Shape> with props for drawing the Circle', async () => {