Compare only decorations offsets in MemoizedText. Code highlighting example improvements. #5271

dsvgit · 2023-01-25T19:53:00Z

Description
Initially, I tried to improve the Code Highlighting example on the Slate.js site. But while working on this, I didn't find a way to do it without changing the slate-react library code.

The Code Highlighting example improvements:

Support syntax highlighting for hierarchical tokens that Prism.js generates. It is relevant for languages like HTML, JSX, TSX, etc...
Tokenize the whole code block at once, instead of each line separately.
Support multiple code blocks on one page. Show how to combine paragraphs and code blocks.
Keep performance after these changes.

Slate-react changes:

Only for keeping the right performance, I had to add changes to the slate-react library. These changes allow returning relative ranges from the decorate function. The basePath field is added to Point. So a range can have the following structure:

const range = { anchor: { basePath: [0, 0], path: [], offset: 0 }, focus: { basePath: [0, 0], path: [], offset: 20 } }

When the function isDecoratorRangeListEqual compares ranges via Range.equals, these fields aren't included in the comparison. It means that even if the basePath is changed, the component isn't re-rendered since other properties are the same.
It is useful when users add or remove code lines above.
It is still compatible with old code because basePath is optional. And if it isn't set everything works as before.

Issue

Example

The example now illustrates multiple code blocks in one editor:

Before:

After:

Html, jsx, tsx syntax highlighted the right way:

Before:

After:

Multiline tokens are supported. When it starts from one line and ends in another:

Before:

After:

If some pieces affect the other, the other one is re-rendered:

Before:

After:

If new elements are added, and they don't affect the elements below they aren't re-rendered. It is possible because now relative ranges are used on slate-react level:

Before:

After (the same result, leaves aren't re-rendered, performance is kept):

Context
Relative points in decorations are primarily intended to prevent re-renders when elements only move their position in the editor while keeping the structure of internal decorations.

Checks

The new code matches the existing patterns and styles.
The tests pass with yarn test.
The linter passes with yarn lint. (Fix errors with yarn fix.)
The relevant examples still work. (Run examples with yarn start.)
You've added a changeset if changing functionality. (Add one with yarn changeset add.)

changeset-bot · 2023-01-25T19:53:03Z

🦋 Changeset detected

Latest commit: 469e76b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
slate-react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

site/examples/code-highlighting.tsx

dsvgit · 2023-01-26T10:15:40Z

site/examples/markdown-preview.tsx

@@ -1,13 +1,11 @@
 import Prism from 'prismjs'
+import 'prismjs/components/prism-markdown'


just minor refactoring: using prism js markdown implementation

packages/slate-react/src/custom-types.ts

packages/slate-react/src/hooks/use-children.tsx

dylans

Thanks @dsvgit this looks good. Will leave it open a few days to give others a chance to give any suggestions or feedback.

dsvgit · 2023-01-26T14:51:29Z

Hey @dylans, great, thanks for taking a look at this

BitPhinix · 2023-01-28T18:14:23Z

@jasonphillips this one might be interesting for you 👀

bryanph · 2023-01-30T11:02:30Z

site/examples/code-highlighting.tsx

+
+// set editor.intervals with intervals of a continuous stream of identical elements
+// set editor.intervalsStarts with set of these intervals start index (for optimization)
+const ExtractIntervals = () => {


could add this as a hook instead perhaps? Edit: oh I see why you did this, no need to make another wrapper this way

So on every editor/selection change this loops over all (top-level only?) editor children. Could you explain what an interval is in this context? Contiguous code lines?

Hey @bryanph. Yes, it's just the contiguous elements intervals calculated on each editor/selection change And it loops only over all top-level editor children. It may have the following data:

{ "paragraph": [[0, 0], [12,14]], "code-line": [[1, 11], [15, 25]] }

So the idea here is to extract additional information based on the current editor's children state. It should be always synced with editor children. That's why these "extractors" were placed inside Slate, but before Editable. It means that all components inside Editable can rely on editor.intervals as on editor.children. I think it actually can be reorganised with hooks in some way. I just wanted to keep it simple.

Actually, it could be optimized by moving these calculations into

useMemo(() => {...; editor.intervals = intervals; }, [editor.children])

hook to prevent calling these calculations when only selection is changed

bryanph · 2023-01-31T10:07:15Z

Great work, a well needed change to the code example. I'm not too sure about the basePath change, maybe someone who has recently put work into the decorations code can add some input on that.

dsvgit · 2023-02-01T22:19:55Z

@bryanph Thanks. I figured out how to keep the same performance without basePath change. I just changed the way how MemoizedText compares decorations. Since only offsets matter for text, I compare only offsets. It looks like just minor changes, that should improve performance but keep the same behaviour as before for other cases.

dsvgit · 2023-02-01T22:24:02Z

Oh, I just clicked multiple times for review request. Didn't think it works this way

dsvgit · 2023-02-01T22:36:50Z

packages/slate-react/src/utils/range-list.ts

-      !Range.equals(range, other) ||
-      range[PLACEHOLDER_SYMBOL] !== other[PLACEHOLDER_SYMBOL] ||
-      !shallowCompare(rangeOwnProps, otherOwnProps)
+      range.anchor.offset !== other.anchor.offset ||


compare only offsets for text inside the isTextDecorationsEqual function

dsvgit · 2023-02-01T22:37:08Z

packages/slate-react/src/components/text.tsx

@@ -79,7 +79,7 @@ const MemoizedText = React.memo(Text, (prev, next) => {
    next.isLast === prev.isLast &&
    next.renderLeaf === prev.renderLeaf &&
    next.text === prev.text &&
-    isDecoratorRangeListEqual(next.decorations, prev.decorations)
+    isTextDecorationsEqual(next.decorations, prev.decorations)


use the isTextDecorationsEqual function that compares only offsets

dylans · 2023-02-08T04:15:05Z

@jasonphillips do you have any feedback on this one before I land it?

jasonphillips · 2023-02-08T04:42:22Z

@bryanph Thanks. I figured out how to keep the same performance without basePath change. I just changed the way how MemoizedText compares decorations. Since only offsets matter for text, I compare only offsets. It looks like just minor changes, that should improve performance but keep the same behaviour as before for other cases.

This is a smart optimization! Slate already sends just the intersection of the decoration down into the text node so, as you said, the path shouldn’t matter for comparison.

BasePath was confusing me a bit and I’m so wary about adding new concepts now that could affect someone’s use case, but this optimization makes perfect sense.

jasonphillips · 2023-02-08T05:09:23Z

But looking at the updated code-highlighting example, I have to say that I'm concerned about having that much complexity in the example.

Examples are meant to illustrate how to do the basics in slate, and it seems that all this additional logic of finding continuous sequences of elements at the top level of the editor etc is going to be very confusing and counterproductive as a code-highlighting example. Each example should ideally be basic and introduce only one concept, which is the feature being illustrated (in this case, just how to use the decorate function).

I'm also unsure of why this approach is chosen (continuous lists of top-level elements) instead of using something like a nested container element, eg. why not have structure editor.children: [ { type: paragraph, ... }, { type: code-container, language: 'typescript', children: [ { type: code-line, ... }, { type: code-line, ... } ]}, ... ] so that the lines of code would be grouped under a parent element, which is where the language selection would be and where the decorate() function would operate.

One of Slate's great advantages over many other editors is to handle truly nested content, after all; and making the rendering of an element depend on its siblings rather than just parents is very tricky to get right--here, your codeline element depends upon checking whether it is the first code line in sequence (prior sibling is not codeline) in order to decide whether to show the language selection.

That said, don't get me wrong--we have cases in our own application that I work on where we likewise have to think of siblings and overall structure of the editor children, and your implementation here looks nice. But I'm concerned that this introduces so much complexity as an example, and it brings in some concepts that are not exactly anti-patterns but definitely not a great place for a beginner to start learning how to use Slate the standard way.

dsvgit · 2023-02-08T10:57:49Z

@jasonphillips thank you for the feedback, most points make sense. I need some time to think about it.

dsvgit · 2023-02-08T10:59:09Z

@dylans I created another PR only with slate-react changes in order to split code-highlighting example and MemoizedText decorations optimisation. #5285

… absolute ranges on passing them into TextComponent

…oring

…alculate absolute ranges on passing them into TextComponent" This reverts commit afa085c.

…elative ranges on passing them to TextComponent

… only if editor.children is changed

…tions

dsvgit · 2023-02-08T17:05:51Z

@jasonphillips , I updated the example, now it should be simpler

For some cases it is a bit harder to support desired behaviour for nested elements, but with flat structure it's just enabled by default. For the current case I agree that it is better to use nested code block structure. I rewrote this part.
With current decorations implementation, I didn't find any ways to achieve the right performance for cases when analysing the whole code block required. It works really slow if I use only decorate function, without ranges precalculations (that nodeToDecorations map provides).
And the last complex thing is the Prism.js integration code. I replaced my code of tokens normalization with util function from prism-react-renderer library. It makes nested tokens flat and group them by lines. This is exactly what we need to map them to ranges for Slate.

dsvgit · 2023-02-08T17:08:06Z

site/examples/code-highlighting.tsx

+    )
+
+    const nodeToDecorations = mergeMaps(
+      ...blockEntries.map(getChildNodeToDecorations)


precalculate editor.nodeToDecorations map to use it inside decorate function then
I didn't find a way to achieve good performance without this precalculation

dsvgit · 2023-02-08T17:11:39Z

site/examples/code-highlighting.tsx

+  }
+}
+
+const getChildNodeToDecorations = ([block, blockPath]: NodeEntry<


this function creates nodeToDecorations map for one code-block element

dsvgit · 2023-02-08T17:12:55Z

site/examples/code-highlighting.tsx

+const useDecorate = (editor: Editor) => {
+  return useCallback(([node, path]) => {
+    if (Element.isElement(node) && node.type === CodeLineType) {
+      const ranges = editor.nodeToDecorations.get(node) || []


just apply precalculated decorations on CodeLine level
We can't apply it on CodeBlockElement level because of bad performance
We can't apply it on Text level because we lost right re-renders

dsvgit · 2023-02-08T17:18:12Z

site/utils/normalize-tokens.ts

+// are always of type "plain".
+// This is not recursive to avoid exceeding the call-stack limit, since it's unclear
+// how nested Prism's tokens can become
+export const normalizeTokens = (


this function needs to apply Prism.js tokens the right way

dylans

Looks like a good round of improvements.

GitHub Actions are showing some playwright test failures (shown via files changed).

dsvgit · 2023-02-09T16:41:50Z

@dylans I just fixed playwright tests. If you are ok with this PR we can just close #5285 because it contains the same changes.

dsvgit marked this pull request as draft January 26, 2023 07:42

dsvgit changed the title ~~Code highlighting example improvements, support relative ranges in decorate function on slate-react level~~ Support relative points in decorations. Code highlighting example improvements. Jan 26, 2023

dsvgit marked this pull request as ready for review January 26, 2023 08:28

dsvgit commented Jan 26, 2023

View reviewed changes

dylans reviewed Jan 26, 2023

View reviewed changes

bryanph reviewed Jan 30, 2023

View reviewed changes

dsvgit requested review from dylans and bryanph and removed request for dylans and bryanph February 1, 2023 22:22

dsvgit commented Feb 1, 2023

View reviewed changes

dsvgit changed the title ~~Support relative points in decorations. Code highlighting example improvements.~~ Compare only decorations offsets in MemoizedText. Code highlighting example improvements. Feb 2, 2023

dsvgit force-pushed the code-highlighting-improvements branch from 56a3245 to d1e14e6 Compare February 7, 2023 12:09

dsvgit mentioned this pull request Feb 8, 2023

Perf: Compare only offsets for MemoizedText decorations #5285

Closed

5 tasks

dsvgit added 4 commits February 8, 2023 21:59

add basePath prop to Range inside slate-react custom types, calculate…

b49ac56

… absolute ranges on passing them into TextComponent

code highlighting example improvements, minor markdown preview refact…

3b9abad

…oring

changeset added

ce62801

Revert "add basePath prop to Range inside slate-react custom types, c…

d60ede9

…alculate absolute ranges on passing them into TextComponent" This reverts commit afa085c.

dsvgit added 10 commits February 8, 2023 21:59

add basePath prop to Point inside slate-react custom types, resolve r…

3baff42

…elative ranges on passing them to TextComponent

Update changeset

7f5771d

linter fixes

d338b8a

remove redundant checks inside renderElement function

e180ab6

custom types fixes for Range and Point in examples

5854683

wrap intervals and ranges extractors in useMemo hook for running them…

8cf3c04

… only if editor.children is changed

revert basePath changes, compare only offsets for MemoizedText decora…

c6fff3d

…tions

use an element as a key in decorations ranges map instead of id

802e157

simplify code highlighting implementation, make code block nested

672f762

fix code-highlighting example, add toolbar code block button

a656d54

dsvgit force-pushed the code-highlighting-improvements branch from 60e1888 to a656d54 Compare February 8, 2023 16:19

dsvgit commented Feb 8, 2023

View reviewed changes

remove redundant code

b2568e1

dylans reviewed Feb 9, 2023

View reviewed changes

fix code highlighting playwright integration test

469e76b

dylans approved these changes Feb 9, 2023

View reviewed changes

dylans merged commit 9635b99 into ianstormtaylor:main Feb 9, 2023

github-actions bot mentioned this pull request Feb 9, 2023

Version Packages #5290

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compare only decorations offsets in MemoizedText. Code highlighting example improvements. #5271

Compare only decorations offsets in MemoizedText. Code highlighting example improvements. #5271

dsvgit commented Jan 25, 2023 •

edited

Loading

changeset-bot bot commented Jan 25, 2023 •

edited

Loading

dsvgit Jan 26, 2023

dylans left a comment

dsvgit commented Jan 26, 2023

BitPhinix commented Jan 28, 2023

bryanph Jan 30, 2023 •

edited

Loading

bryanph Jan 30, 2023 •

edited

Loading

dsvgit Jan 30, 2023

dsvgit Jan 30, 2023 •

edited

Loading

bryanph commented Jan 31, 2023

dsvgit commented Feb 1, 2023

dsvgit commented Feb 1, 2023

dsvgit Feb 1, 2023 •

edited

Loading

dsvgit Feb 1, 2023 •

edited

Loading

dylans commented Feb 8, 2023

jasonphillips commented Feb 8, 2023

jasonphillips commented Feb 8, 2023

dsvgit commented Feb 8, 2023

dsvgit commented Feb 8, 2023

dsvgit commented Feb 8, 2023

dsvgit Feb 8, 2023

dsvgit Feb 8, 2023

dsvgit Feb 8, 2023 •

edited

Loading

dsvgit Feb 8, 2023 •

edited

Loading

dylans left a comment

dsvgit commented Feb 9, 2023

		@@ -1,13 +1,11 @@
		import Prism from 'prismjs'
		import 'prismjs/components/prism-markdown'

Compare only decorations offsets in MemoizedText. Code highlighting example improvements. #5271

Compare only decorations offsets in MemoizedText. Code highlighting example improvements. #5271

Conversation

dsvgit commented Jan 25, 2023 • edited Loading

changeset-bot bot commented Jan 25, 2023 • edited Loading

🦋 Changeset detected

dsvgit Jan 26, 2023

Choose a reason for hiding this comment

dylans left a comment

Choose a reason for hiding this comment

dsvgit commented Jan 26, 2023

BitPhinix commented Jan 28, 2023

bryanph Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

bryanph Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

dsvgit Jan 30, 2023

Choose a reason for hiding this comment

dsvgit Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

bryanph commented Jan 31, 2023

dsvgit commented Feb 1, 2023

dsvgit commented Feb 1, 2023

dsvgit Feb 1, 2023 • edited Loading

Choose a reason for hiding this comment

dsvgit Feb 1, 2023 • edited Loading

Choose a reason for hiding this comment

dylans commented Feb 8, 2023

jasonphillips commented Feb 8, 2023

jasonphillips commented Feb 8, 2023

dsvgit commented Feb 8, 2023

dsvgit commented Feb 8, 2023

dsvgit commented Feb 8, 2023

dsvgit Feb 8, 2023

Choose a reason for hiding this comment

dsvgit Feb 8, 2023

Choose a reason for hiding this comment

dsvgit Feb 8, 2023 • edited Loading

Choose a reason for hiding this comment

dsvgit Feb 8, 2023 • edited Loading

Choose a reason for hiding this comment

dylans left a comment

Choose a reason for hiding this comment

dsvgit commented Feb 9, 2023

dsvgit commented Jan 25, 2023 •

edited

Loading

changeset-bot bot commented Jan 25, 2023 •

edited

Loading

bryanph Jan 30, 2023 •

edited

Loading

bryanph Jan 30, 2023 •

edited

Loading

dsvgit Jan 30, 2023 •

edited

Loading

dsvgit Feb 1, 2023 •

edited

Loading

dsvgit Feb 1, 2023 •

edited

Loading

dsvgit Feb 8, 2023 •

edited

Loading

dsvgit Feb 8, 2023 •

edited

Loading