Skip to content

Commit

Permalink
Make ART Concurrent if Legacy Mode is disabled
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sebmarkbage committed Apr 2, 2024
1 parent 48ec17b commit 3a3e6ee
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 14 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
27 changes: 18 additions & 9 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,27 @@ 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').act;

// Isolate DOM renderer.
jest.resetModules();
// share isomorphic
jest.mock('scheduler', () => Scheduler);
jest.mock('react', () => React);
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 +74,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 +88,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 +418,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 Down Expand Up @@ -447,9 +454,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 3a3e6ee

Please sign in to comment.