Skip to content

Commit 3e3589f

Browse files
committed
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.
1 parent 48ec17b commit 3e3589f

File tree

2 files changed

+41
-17
lines changed

2 files changed

+41
-17
lines changed

packages/react-art/src/ReactART.js

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,17 @@
77

88
import * as React from 'react';
99
import ReactVersion from 'shared/ReactVersion';
10-
import {LegacyRoot} from 'react-reconciler/src/ReactRootTags';
10+
import {LegacyRoot, ConcurrentRoot} from 'react-reconciler/src/ReactRootTags';
1111
import {
1212
createContainer,
1313
updateContainer,
1414
injectIntoDevTools,
15+
flushSync,
1516
} from 'react-reconciler/src/ReactFiberReconciler';
1617
import Transform from 'art/core/transform';
1718
import Mode from 'art/modes/current';
1819
import FastNoSideEffects from 'art/modes/fast-noSideEffects';
20+
import {disableLegacyMode} from 'shared/ReactFeatureFlags';
1921

2022
import {TYPES, childrenAsString} from './ReactARTInternals';
2123

@@ -68,13 +70,17 @@ class Surface extends React.Component {
6870

6971
this._mountNode = createContainer(
7072
this._surface,
71-
LegacyRoot,
73+
disableLegacyMode ? ConcurrentRoot : LegacyRoot,
7274
null,
7375
false,
7476
false,
7577
'',
7678
);
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+
});
7884
}
7985

8086
componentDidUpdate(prevProps, prevState) {
@@ -84,15 +90,23 @@ class Surface extends React.Component {
8490
this._surface.resize(+props.width, +props.height);
8591
}
8692

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+
});
8898

8999
if (this._surface.render) {
90100
this._surface.render();
91101
}
92102
}
93103

94104
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+
});
96110
}
97111

98112
render() {

packages/react-art/src/__tests__/ReactART-test.js

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111

1212
'use strict';
1313

14-
import * as React from 'react';
14+
const React = require('react');
15+
const Scheduler = require('scheduler');
1516

1617
import * as ReactART from 'react-art';
1718
import ARTSVGMode from 'art/modes/svg';
@@ -22,22 +23,28 @@ import Circle from 'react-art/Circle';
2223
import Rectangle from 'react-art/Rectangle';
2324
import Wedge from 'react-art/Wedge';
2425

26+
const {act, waitFor} = require('internal-test-utils');
27+
2528
// Isolate DOM renderer.
2629
jest.resetModules();
30+
// share isomorphic
31+
jest.mock('scheduler', () => Scheduler);
32+
jest.mock('react', () => React);
33+
const ReactDOM = require('react-dom');
2734
const ReactDOMClient = require('react-dom/client');
28-
let act = require('internal-test-utils').act;
2935

3036
// Isolate the noop renderer
3137
jest.resetModules();
38+
// share isomorphic
39+
jest.mock('scheduler', () => Scheduler);
40+
jest.mock('react', () => React);
3241
const ReactNoop = require('react-noop-renderer');
33-
const Scheduler = require('scheduler');
3442

3543
let Group;
3644
let Shape;
3745
let Surface;
3846
let TestComponent;
3947

40-
let waitFor;
4148
let groupRef;
4249

4350
const Missing = {};
@@ -68,6 +75,11 @@ describe('ReactART', () => {
6875
let container;
6976

7077
beforeEach(() => {
78+
jest.resetModules();
79+
// share isomorphic
80+
jest.mock('scheduler', () => Scheduler);
81+
jest.mock('react', () => React);
82+
7183
container = document.createElement('div');
7284
document.body.appendChild(container);
7385

@@ -77,8 +89,6 @@ describe('ReactART', () => {
7789
Shape = ReactART.Shape;
7890
Surface = ReactART.Surface;
7991

80-
({waitFor} = require('internal-test-utils'));
81-
8292
groupRef = React.createRef();
8393
TestComponent = class extends React.Component {
8494
group = groupRef;
@@ -409,8 +419,6 @@ describe('ReactART', () => {
409419
);
410420
}
411421

412-
// Using test renderer instead of the DOM renderer here because async
413-
// testing APIs for the DOM renderer don't exist.
414422
ReactNoop.render(
415423
<CurrentRendererContext.Provider value="Test">
416424
<Yield value="A" />
@@ -423,7 +431,9 @@ describe('ReactART', () => {
423431
await waitFor(['A']);
424432

425433
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(() => {
427437
root.render(
428438
<Surface>
429439
<LogCurrentRenderer />
@@ -434,8 +444,6 @@ describe('ReactART', () => {
434444
);
435445
});
436446

437-
expect(ops).toEqual([null, 'ART']);
438-
439447
ops = [];
440448
await waitFor(['B', 'C']);
441449

@@ -447,9 +455,11 @@ describe('ReactARTComponents', () => {
447455
let ReactTestRenderer;
448456
beforeEach(() => {
449457
jest.resetModules();
458+
// share isomorphic
459+
jest.mock('scheduler', () => Scheduler);
460+
jest.mock('react', () => React);
450461
// Isolate test renderer.
451462
ReactTestRenderer = require('react-test-renderer');
452-
act = require('internal-test-utils').act;
453463
});
454464

455465
it('should generate a <Shape> with props for drawing the Circle', async () => {

0 commit comments

Comments
 (0)