Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RichText: Reuse DOM document across calls to createEmpty #12402

Merged
merged 4 commits into from
Nov 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/rich-text/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 3.0.3 (Unreleased)

### Internal

- Internal performance optimizations to avoid excessive expensive creation of DOM documents.

## 3.0.2 (2018-11-21)

## 3.0.1 (2018-11-20)
Expand Down
18 changes: 15 additions & 3 deletions packages/rich-text/src/create-element.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
/**
* Parse the given HTML into a body element.
*
* Note: The current implementation will return a shared reference, reset on
* each call to `createElement`. Therefore, you should not hold a reference to
* the value to operate upon asynchronously, as it may have unexpected results.
*
* @param {HTMLDocument} document The HTML document to use to parse.
* @param {string} html The HTML to parse.
*
* @return {HTMLBodyElement} Body element with parsed HTML.
*/
export function createElement( { implementation }, html ) {
const { body } = implementation.createHTMLDocument( '' );
body.innerHTML = html;
return body;
// Because `createHTMLDocument` is an expensive operation, and with this
// function being internal to `rich-text` (full control in avoiding a risk
// of asynchronous operations on the shared reference), a single document
// is reused and reset for each call to the function.
if ( ! createElement.body ) {
createElement.body = implementation.createHTMLDocument( '' ).body;
}

createElement.body.innerHTML = html;

return createElement.body;
}
4 changes: 2 additions & 2 deletions packages/rich-text/src/test/to-dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ describe( 'applyValue', () => {

cases.forEach( ( { current, future, description, movedCount } ) => {
it( description, () => {
const body = createElement( document, current );
const futureBody = createElement( document, future );
const body = createElement( document, current ).cloneNode( true );
const futureBody = createElement( document, future ).cloneNode( true );
const childNodes = Array.from( futureBody.childNodes );
applyValue( futureBody, body );
const count = childNodes.reduce( ( acc, { parentNode } ) => {
Expand Down
16 changes: 12 additions & 4 deletions packages/rich-text/src/to-dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/

import { toTree } from './to-tree';
import { createElement } from './create-element';

/**
* Browser dependencies
Expand Down Expand Up @@ -58,10 +59,17 @@ function getNodeByPath( node, path ) {
};
}

function createEmpty() {
const { body } = document.implementation.createHTMLDocument( '' );
return body;
}
/**
* Returns a new instance of a DOM tree upon which RichText operations can be
* applied.
*
* Note: The current implementation will return a shared reference, reset on
* each call to `createEmpty`. Therefore, you should not hold a reference to
* the value to operate upon asynchronously, as it may have unexpected results.
*
* @return {WPRichTextTree} RichText tree.
*/
const createEmpty = () => createElement( document, '' );

function append( element, child ) {
if ( typeof child === 'string' ) {
Expand Down