-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Fix stale decorations #4876
Merged
Merged
Fix stale decorations #4876
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: ad1b94f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
The base typescript config uses ESNext as target, so presumably the latest node should be used.
Merged
5 tasks
DougReeder
pushed a commit
to DougReeder/slate
that referenced
this pull request
Apr 3, 2022
* test changes * fix decoration not updating * Add changeset * Fix lint issues * Tests with earlier version of Node.js * Bump node version on CI The base typescript config uses ESNext as target, so presumably the latest node should be used. Co-authored-by: Dylan Schiemann <dylan@dojotoolkit.org>
5 tasks
jasonphillips
added a commit
to jasonphillips/slate
that referenced
this pull request
May 25, 2022
This reverts commit 1b205c0.
5 tasks
dylans
pushed a commit
that referenced
this pull request
May 25, 2022
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Given that all relevant functions used to memoize an element do not change, when the editor re-renders no new decorations will be applied unless the entire tree from root to element contains a change. This leads to "decorate" being called only on the top elements, not any nested ones.
This PR resolves that by generating all decorations upfront instead of lazily as the tree is walked.
Context
The decorations prior to this PR are generated lazily, as the tree is walked. First we generate decoration for the editor, then the children of editor, then the children of children etc... Traversal is stopped however unless the following memo is invalidated:
If only looking at the decorations of the current node, and not the children, the memo will not be invalidated and decorations will not be updated. This is resolved by generating everything upfront. The memo is then invalidated by providing the decorations for the intersection for the current range, as follows:
which contain both the current element and children.
Checks
yarn test
.yarn lint
. (Fix errors withyarn fix
.)yarn start
.)yarn changeset add
.)