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

Don't throw if getPreviousSiblings can't find a parent #3543

Merged
merged 1 commit into from
Dec 13, 2022
Merged

Conversation

acywatson
Copy link
Contributor

This method used to safely return an empty list instead of throwing if the parent isn't found. This changes it back to that approach, so the API matches similar methods and the linked list change remains an implementation detail.

@vercel
Copy link

vercel bot commented Dec 12, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
lexical ✅ Ready (Inspect) Visit Preview Dec 12, 2022 at 11:10PM (UTC)
lexical-playground ✅ Ready (Inspect) Visit Preview Dec 12, 2022 at 11:10PM (UTC)

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 12, 2022
@acywatson acywatson changed the title Do not throw if getPreviousSiblings cannot find a parent Don't throw if getPreviousSiblings can't find a parent Dec 12, 2022
@fantactuka
Copy link
Contributor

Curious if it's possible for node to have null parent but non-null siblings. If that's the case then getPrev/NextSiblings will work differently. That said, should we have check for parent in getNextSiblings too to avoid it?

@acywatson
Copy link
Contributor Author

Curious if it's possible for node to have null parent but non-null siblings. If that's the case then getPrev/NextSiblings will work differently. That said, should we have check for parent in getNextSiblings too to avoid it?

I suppose anything is possible? We never seemed to have that issue before, when this method worked this way.

Maybe the best solution ultimately is to make some of these methods invariant if they're called on RootNode. It's pretty nonsensical. But then you have to check for RootNode everywhere.

Copy link
Collaborator

@trueadm trueadm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I think I did this by accident

@acywatson
Copy link
Contributor Author

Makes sense. I think I did this by accident

I figured that might be the case - thanks!

@acywatson acywatson merged commit fde8077 into main Dec 13, 2022
@trueadm trueadm mentioned this pull request Dec 17, 2022
@trueadm trueadm deleted the fix-siblings branch December 18, 2022 00:52
abelsj60 pushed a commit to abelsj60/lexical that referenced this pull request Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants