Skip to content

Commit

Permalink
Use flushSync to avoid render lag (fixes MBS-12424)
Browse files Browse the repository at this point in the history 
React v18 uses a new method of rendering that can be interrupted and
doesn't block the page from performing other work. This can make updates
appear laggy where we're trying to render many successive roots -- there
is a noticeable delay between rendering each root where React yields its
time. (In the case of MBS-12424, each "root" would be a track artist
credit container.) This isn't really a problem with React, but a side
effect of how we're gluing it into non-React code. Normally we wouldn't
have so many roots.

In order to make our haphazard way of rendering things into non-React
pages appear more snappy, I'm wrapping all calls to `render` and
`hydrateRoot` with `flushSync`, which causes all pending updates to the
DOM to occur immediately. This keeps that behavior in line with how
React v17 worked.

Wrapping `hydrateRoot` in particular may affect the timing of when user
scripts are allowed to interact with the page and avoid hydration
errors.
  • Loading branch information
@mwiencek
mwiencek committed Jun 2, 2022
1 parent 9fbd4e7 commit ac2b1b0
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 90 deletions.
25 changes: 14 additions & 11 deletions root/static/scripts/common/MB/Control/Autocomplete.js
View file
Expand Up @@ -9,6 +9,7 @@
import he from 'he';
import $ from 'jquery';
import ko from 'knockout';
import {flushSync} from 'react-dom';
import * as ReactDOMClient from 'react-dom/client';

import AddEntityDialog, {
Expand Down Expand Up @@ -507,17 +508,19 @@ $.widget('mb.entitylookup', $.ui.autocomplete, {

root = ReactDOMClient.createRoot(containerNode);
/* eslint-disable react/jsx-no-bind */
root.render(
<AddEntityDialog
callback={(item) => {
self.options.select(null, {item});
closeAndReturnFocus();
}}
close={closeAndReturnFocus}
entityType={entity}
name={self._value()}
/>,
);
flushSync(() => {
root.render(
<AddEntityDialog
callback={(item) => {
self.options.select(null, {item});
closeAndReturnFocus();
}}
close={closeAndReturnFocus}
entityType={entity}
name={self._value()}
/>,
);
});
/* eslint-enable react/jsx-no-bind */
},
});
Expand Down
21 changes: 13 additions & 8 deletions root/static/scripts/edit/check-duplicates.js
View file
Expand Up @@ -8,6 +8,7 @@

import $ from 'jquery';
import ko from 'knockout';
import {flushSync} from 'react-dom';
import * as ReactDOMClient from 'react-dom/client';

import MB from '../common/MB';
Expand Down Expand Up @@ -40,18 +41,22 @@ var requestPending = validation.errorField(ko.observable(false));
function renderDuplicates(name, duplicates, dupeRoot) {
needsConfirmation(true);

dupeRoot.render(
<PossibleDuplicates
duplicates={duplicates}
name={name}
onCheckboxChange={event => isConfirmed(event.target.checked)}
/>,
);
flushSync(() => {
dupeRoot.render(
<PossibleDuplicates
duplicates={duplicates}
name={name}
onCheckboxChange={event => isConfirmed(event.target.checked)}
/>,
);
});
}

function unmountDuplicates(dupeRoot) {
needsConfirmation(false);
dupeRoot.render(null);
flushSync(() => {
dupeRoot.render(null);
});
}

function sortPlaceDuplicates(duplicates) {
Expand Down
59 changes: 31 additions & 28 deletions root/static/scripts/edit/components/ArtistCreditEditor.js
View file
Expand Up @@ -9,6 +9,7 @@
import $ from 'jquery';
import ko from 'knockout';
import * as React from 'react';
import {flushSync} from 'react-dom';
import * as ReactDOMClient from 'react-dom/client';
import mutate from 'mutate-cow';

Expand Down Expand Up @@ -236,34 +237,36 @@ class ArtistCreditEditor extends React.Component {
$bubble.data('react-root', bubbleRoot);
}

bubbleRoot.render(
<ArtistCreditBubble
addName={this.addName}
artistCredit={this.state.artistCredit}
copyArtistCredit={this.copyArtistCredit}
done={this.done}
hide={this.hide}
initialArtistText={this.initialArtistText}
onNameChange={this.handleNameChange}
pasteArtistCredit={this.pasteArtistCredit}
removeName={this.removeName}
renderCallback={
show ? (() => {
this.positionBubble();

if (!bubbleWasVisible) {
$bubble.find(':input:eq(0)').focus();
$('#change-matching-artists').prop('checked', false);
}

if (callback) {
callback();
}
}) : null
}
{...this.props}
/>,
);
flushSync(() => {
bubbleRoot.render(
<ArtistCreditBubble
addName={this.addName}
artistCredit={this.state.artistCredit}
copyArtistCredit={this.copyArtistCredit}
done={this.done}
hide={this.hide}
initialArtistText={this.initialArtistText}
onNameChange={this.handleNameChange}
pasteArtistCredit={this.pasteArtistCredit}
removeName={this.removeName}
renderCallback={
show ? (() => {
this.positionBubble();

if (!bubbleWasVisible) {
$bubble.find(':input:eq(0)').focus();
$('#change-matching-artists').prop('checked', false);
}

if (callback) {
callback();
}
}) : null
}
{...this.props}
/>,
);
});
}

hide(stealFocus = true) {
Expand Down
9 changes: 6 additions & 3 deletions root/static/scripts/edit/components/forms.js
View file
Expand Up @@ -7,6 +7,7 @@
*/

import ko from 'knockout';
import {flushSync} from 'react-dom';
import * as ReactDOMClient from 'react-dom/client';

import MB from '../../common/MB';
Expand Down Expand Up @@ -36,9 +37,11 @@ MB.initializeArtistCredit = function (form, initialArtistCredit) {

const container = document.getElementById('artist-credit-editor');
const root = ReactDOMClient.createRoot(container);
root.render(
<FormRowArtistCredit entity={source} form={form} />,
);
flushSync(() => {
root.render(
<FormRowArtistCredit entity={source} form={form} />,
);
});

source.artistCredit.subscribe((artistCredit) => {
$('table.artist-credit-editor', container)
Expand Down
23 changes: 13 additions & 10 deletions root/static/scripts/edit/externalLinks.js
View file
Expand Up @@ -12,6 +12,7 @@ import punycode from 'punycode';
import $ from 'jquery';
import ko from 'knockout';
import * as React from 'react';
import {flushSync} from 'react-dom';
import * as ReactDOMClient from 'react-dom/client';

import {
Expand Down Expand Up @@ -1777,16 +1778,18 @@ MB.createExternalLinksEditor = function (options: InitialOptionsT) {

const root = ReactDOMClient.createRoot(options.mountPoint);
const externalLinksEditorRef = React.createRef();
root.render(
<ExternalLinksEditor
errorObservable={errorObservable}
initialLinks={initialLinks}
isNewEntity={!sourceData.id}
ref={externalLinksEditorRef}
sourceType={sourceData.entityType}
typeOptions={typeOptions}
/>,
);
flushSync(() => {
root.render(
<ExternalLinksEditor
errorObservable={errorObservable}
initialLinks={initialLinks}
isNewEntity={!sourceData.id}
ref={externalLinksEditorRef}
sourceType={sourceData.entityType}
typeOptions={typeOptions}
/>,
);
});
return externalLinksEditorRef;
};

Expand Down
14 changes: 11 additions & 3 deletions root/static/scripts/release-editor/bindingHandlers.js
View file
Expand Up @@ -9,6 +9,7 @@
import $ from 'jquery';
import ko from 'knockout';
import * as React from 'react';
import {flushSync} from 'react-dom';
import * as ReactDOMClient from 'react-dom/client';

import {reduceArtistCredit} from '../common/immutable-entities';
Expand Down Expand Up @@ -143,9 +144,16 @@ ko.bindingHandlers.artistCreditEditor = {
if (!entity.artistCreditEditorInst) {
entity.artistCreditEditorInst = React.createRef();
}
root.render(
<ArtistCreditEditor ref={entity.artistCreditEditorInst} {...props} />,
);
/*
* MBS-12424: Due to React v18's asynchronous method of rendering, there
* is a noticeable lag in displaying the artist credit editor of each
* track unless we flush updates immediately.
*/
flushSync(() => {
root.render(
<ArtistCreditEditor ref={entity.artistCreditEditorInst} {...props} />,
);
});
},
};

Expand Down
41 changes: 22 additions & 19 deletions root/static/scripts/work/edit.js
View file
Expand Up @@ -10,6 +10,7 @@
import $ from 'jquery';
import ko from 'knockout';
import mutate from 'mutate-cow';
import {flushSync} from 'react-dom';
import * as ReactDOMClient from 'react-dom/client';
import {createStore} from 'redux';

Expand Down Expand Up @@ -301,25 +302,27 @@ function renderWorkLanguages() {
const form: WorkForm = store.getState();
const selectedLanguageIds =
form.field.languages.field.map(lang => String(lang.value));
workLanguagesRoot.render(
<FormRowSelectList
addId="add-language"
addLabel={l('Add Language')}
getSelectField={getSelectField}
hideAddButton={
selectedLanguageIds.includes(String(LANGUAGE_MUL_ID)) ||
selectedLanguageIds.includes(String(LANGUAGE_ZXX_ID))
}
label={l('Lyrics Languages')}
onAdd={addLanguage}
onEdit={editLanguage}
onRemove={removeLanguage}
options={workLanguageOptions}
removeClassName="remove-language"
removeLabel={l('Remove Language')}
repeatable={form.field.languages}
/>,
);
flushSync(() => {
workLanguagesRoot.render(
<FormRowSelectList
addId="add-language"
addLabel={l('Add Language')}
getSelectField={getSelectField}
hideAddButton={
selectedLanguageIds.includes(String(LANGUAGE_MUL_ID)) ||
selectedLanguageIds.includes(String(LANGUAGE_ZXX_ID))
}
label={l('Lyrics Languages')}
onAdd={addLanguage}
onEdit={editLanguage}
onRemove={removeLanguage}
options={workLanguageOptions}
removeClassName="remove-language"
removeLabel={l('Remove Language')}
repeatable={form.field.languages}
/>,
);
});
}

store.subscribe(renderWorkLanguages);
Expand Down
24 changes: 16 additions & 8 deletions root/utility/hydrate.js
View file
Expand Up @@ -9,6 +9,7 @@

import mutate from 'mutate-cow';
import * as React from 'react';
import {flushSync} from 'react-dom';
import * as ReactDOMClient from 'react-dom/client';
import * as Sentry from '@sentry/browser';

Expand Down Expand Up @@ -122,14 +123,21 @@ export default function hydrate<
if (__DEV__) {
checkForUnsanitizedEditorData((props: any));
}
ReactDOMClient.hydrateRoot(
root,
<React.StrictMode>
<SanitizedCatalystContext.Provider value={$c}>
<Component $c={$c} {...props} />
</SanitizedCatalystContext.Provider>
</React.StrictMode>,
);
/*
* Flush updates to the DOM immediately to try and avoid hydration
* errors due to user scripts modifying the page. This is ultimately
* affected by the order in which the scripts run, though.
*/
flushSync(() => {
ReactDOMClient.hydrateRoot(
root,
<React.StrictMode>
<SanitizedCatalystContext.Provider value={$c}>
<Component $c={$c} {...props} />
</SanitizedCatalystContext.Provider>
</React.StrictMode>,
);
});
}
}
});
Expand Down

0 comments on commit ac2b1b0

Please sign in to comment.