Skip to content

Commit

Permalink
Make ART Concurrent if Legacy Mode is disabled (facebook#28662)
Browse files Browse the repository at this point in the history
Pulling this out of facebook#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.
  • Loading branch information
sebmarkbage authored Apr 2, 2024
1 parent 8cb6a1c commit 5fcaa0a
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 17 deletions.
24 changes: 19 additions & 5 deletions packages/react-art/src/ReactART.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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) {
Expand All @@ -84,15 +90,23 @@ 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();
}
}

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() {
Expand Down
34 changes: 22 additions & 12 deletions packages/react-art/src/__tests__/ReactART-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 = {};
Expand Down Expand Up @@ -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);

Expand All @@ -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;
Expand Down Expand Up @@ -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(
<CurrentRendererContext.Provider value="Test">
<Yield value="A" />
Expand All @@ -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(
<Surface>
<LogCurrentRenderer />
Expand All @@ -434,8 +444,6 @@ describe('ReactART', () => {
);
});

expect(ops).toEqual([null, 'ART']);

ops = [];
await waitFor(['B', 'C']);

Expand All @@ -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 <Shape> with props for drawing the Circle', async () => {
Expand Down

0 comments on commit 5fcaa0a

Please sign in to comment.