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

Add Node.getIf method #4845

Closed
thesunny opened this issue Feb 18, 2022 · 1 comment · Fixed by #5723
Closed

Add Node.getIf method #4845

thesunny opened this issue Feb 18, 2022 · 1 comment · Fixed by #5723
Assignees
Labels

Comments

@thesunny
Copy link
Collaborator

Problem
If we need to check if a node exists at a given path and return it, we currently have to do this in several steps: (1) check to see if the Node exists using Node.has. (2) check the return value and if it's false we end and if it's true we (3) call Node.get. We need to do it in this order because if we call Node.get and the path doesn't exist, Slate will throw an error.

The code for these two methods are nearly identical and both require us to loop through the Document to get to the path.

Solution
Instead, we can just add as a method Node.getIf which returns the Node if it exists or undefined if it does not exist.

This has the performance benefit of not iterating through the DOM twice and also simplifies usage of Slate.

A benefit of this is that we can have Node.get and Node.has call Node.getIf and the addition of this feature may actually result in a net decrease of code.

Alternatives
We can do a try/catch with Node.get but this complicates the code rather than simplifies it.

Context
I think this is a common use case. For example, if we want to find out what Node is before or after the current node, we would be currently using Node.has followed by Node.get passing in the path of Path.previous or Path.next. Adding this method means we only have to use Node.getIf.

@thesunny thesunny self-assigned this Feb 18, 2022
@LeonSo7
Copy link

LeonSo7 commented May 30, 2024

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants