-
Notifications
You must be signed in to change notification settings - Fork 3.3k
feat: implement findPath without relying on DOM #5800
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: f5e771f The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
| if (!parent) { | ||
| throw new Error( | ||
| `Unable to find the path for Slate node (parent not found): ${Scrubber.stringify( | ||
| node | ||
| )}` | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could occur during normal use if editor.children is modified in a way that bypasses normaliseNode, such as when setting editor.children directly or when finding the path of a newly added node inside withoutNormalizing. I'd suggest traversing the full tree to repair the cache in this case, and only throw an error if the parent still cannot be found. In addition, the withoutNormalizing case can be mitigated by updating the cache in editor.apply instead of normalizeNode.
| throw new Error( | ||
| `Unable to find the path for Slate node (node is not child of its parent): ${Scrubber.stringify( | ||
| node | ||
| )}` | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar situation to above
|
Can slate be virtualized? |
Current Status
I call
editor.findPathfromDOMEditor.findPathand compare the new implementation's returned path with legacy implementation's returned path.Currently all tests can pass and original calls are not migrated yet. Need to discuss what kind of test cases are needed.
Description
This PR aims at replacing
ReactEditor.findPathwithEditor.findPathCurrently, two weakmaps
NODE_TO_PARENTandNODE_TO_INDEXare set when rendering the React component tree. These two weakmaps help compute a node's path.However, if we are going to support virtualization, then not all components will be rendered, which means some nodes' information cannot be collected. Therefore, we'll not rely on React to compute the path of each element.
In this PR, we create an entry
editor._cacheswhich holds internal caches. Currently it only containsnodeToParent, which works exactly the same likeNODE_TO_PARENTbefore.We patch
normalizeNodefunction so that when a node becomes dirty, then we update all of its children's parent to this new node.NODE_TO_INDEXno longer exists, we'll directly callparent.children.indexOf. Since it is a native API, we assume it is fast enough.When finding a path, it would find parents until root (editor), then get all levels of indices, then construct the whole path.
Checks
yarn test.yarn lint. (Fix errors withyarn fix.)yarn start.)yarn changeset add.)