draft-js-utils
draft-js-utils copied to clipboard
Fix weird performance issue
Hi,
Thanks for the great react-rte
! We noticed a few performance issues with the underlying draft-js-import-html
and draft-js-import-element
libraries. I tried to fix it for "our use case" and added tests that make sure large/weird HTML structures can be parsed.
Existing tests still work fine 🎉
//cc @HenrikFricke
This breaks other untested code/cases ;(
@sbstjn is this ready to merge?
This fix breaks parsing.
For this input:
<p style="">
<span>Line one.</span>
</p>
<p style="">
<span>Line two </span><a href="/foo/bar"><span>link</span></a><span>.</span>
</p>
<p style="">
<span>Line three </span><a href="https://foo.bar"><span>link one</span></a><span> and </span><a href="https://foo2.bar"><span>link two</span></a><span>.</span>
</p>
Notice the text " and "
between the links in the third paragraph.
characterMeta
is incorrectly built for them and the output marks " and "
as part of the second "link" entity which in turn results in DraftJS rendering it as <a href...>link</a>
after which it completely omits the remaining five characters.
The bug this is trying to fix is in the processText
method: var seq = Repeat(charMetadata, text.length);
should actually be var seq = Repeat(charMetadata, 1);
. I can't figure out why a Repeat
was used here in the first place, but repeating the same sequence of characters the same times as it's length is for sure not correct. Replacing that with var seq = Repeat(charMetadata, 1);
fixes the original bug and the fix here (if (!characterMeta.isSuperset(textFragment.characterMeta)) {
) is not required.
Later: my fix does not work.
But this patch does:
diff --git a/node_modules/draft-js-import-element/esm/stateFromElement.js b/node_modules/draft-js-import-element/esm/stateFromElement.js
index eccabfd..d14b574 100644
--- a/node_modules/draft-js-import-element/esm/stateFromElement.js
+++ b/node_modules/draft-js-import-element/esm/stateFromElement.js
@@ -513,8 +513,9 @@ function collapseWhiteSpace(text, characterMeta) {
text = _trimTrailingSpace.text;
characterMeta = _trimTrailingSpace.characterMeta;
- var i = text.length;
+ characterMeta = characterMeta.toArray();
+ var i = text.length;
while (i--) {
if (text.charAt(i) === ' ' && text.charAt(i - 1) === ' ') {
text = text.slice(0, i) + text.slice(i + 1);
@@ -522,6 +523,8 @@ function collapseWhiteSpace(text, characterMeta) {
}
} // There could still be one space on either side of a softbreak.
+ characterMeta = Seq(characterMeta);
+
var _replaceTextWithMeta = replaceTextWithMeta({
text: text,
@@ -561,14 +564,16 @@ function canHaveDepth(blockType) {
function concatFragments(fragments) {
var text = '';
- var characterMeta = Seq();
- fragments.forEach(function (textFragment) {
- text = text + textFragment.text;
- characterMeta = characterMeta.concat(textFragment.characterMeta);
- });
+
+ var characterMeta = [];
+ for (var i = 0; i < fragments.length; i++) {
+ text = text + fragments[i].text;
+ characterMeta.push(...fragments[i].characterMeta.toArray());
+ }
+
return {
text: text,
- characterMeta: characterMeta
+ characterMeta: Seq(characterMeta)
};
}