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

[lexical][lexical-utils][lexical-selection][lexical-table] Feature: NodeCaret abstraction for traversals and ranges #7046

Merged
merged 82 commits into from
Feb 8, 2025

Conversation

etrepum
Copy link
Collaborator

@etrepum etrepum commented Jan 13, 2025

Description

A fair amount of bugs and complexity in our code results from ambiguity and awkwardness around traversing the node tree, as well as issues around coordinates becoming stale after mutations. This is a proposal for a new set of abstractions that can be used in place of the typical PointType and RangeSelection to work through these sorts of traversal use cases with hopefully fewer bugs.

There are three major deficiencies that the design of NodeCaret hopes to address:

  • PointType is not directional, and it's also not possible to infer a direction from a collapsed RangeSelection.
  • Dealing with the borders of an ElementNode is full of quirks, especially when empty, frequently causing over-selection or under-selection or just having to frequently branch for empty and non-empty cases.
  • Instability of PointType when doing anything "before" the point (e.g. inserting a node before a point "moves" the logical location of the point to the newly inserted node)

Docs:

I can write up some flow types when/if we move forward with the proposal to reduce the maintenance burden if anything changes before acceptance.

Cost

The current estimate looks like it's currently at 31.55 KB (+8.56% 🔺) which is +~2.5kB for the production cjs build. There will be some follow-up PRs to reduce this further with additional refactoring to use this API more internally.

Other changes

I didn't refactor everything with NodeCaret but chose a few places that would be straightforward and/or make it easier to fix a few bugs. More will come later which should reduce code size.

lexical

  • Refactor RangeSelection.removeText with new algorithm based on CaretRange
  • Clean up TextNode.splitText algorithm
  • Fix over-selection in getNodes() (refactor based on NodeCaret coming later)

@lexical/selection

  • Fixed $forEachSelectedTextNode to work with types other than RangeSelection
  • Export $patchStyle for updating the style of an individual TextNode or collapsed RangeSelection
  • Export new $ensureForwardRangeSelection function to flip anchor/focus when backwards
  • $setBlocksType: Add new $afterCreateElement argument to allow user to specify how properties propagate to the new block (e.g. format and indent by default - fixes an indent related bug)
  • Export new $copyBlockFormatIndent used as new $setBlocksType optional argument default

@lexical/table

  • Export existing $createTableSelectionFrom
  • Add more unit tests for TableSelection

@lexical/utils

  • Refactor $dfs and related functions with NodeCaret implementations

Closes #7076
Closes #7081

Copy link

vercel bot commented Jan 13, 2025

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

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 8, 2025 2:24am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 8, 2025 2:24am

@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 Jan 13, 2025

This comment was marked as outdated.

@etrepum etrepum changed the title [WIP][lexical[lexical-utils] Feature: NodeCaret abstraction for traversals and ranges [WIP][Breaking Change][lexical[lexical-utils] Feature: NodeCaret abstraction for traversals and ranges Jan 14, 2025
@etrepum etrepum changed the title [WIP][Breaking Change][lexical[lexical-utils] Feature: NodeCaret abstraction for traversals and ranges [WIP][lexical[lexical-utils] Feature: NodeCaret abstraction for traversals and ranges Jan 14, 2025
zurfyx
zurfyx previously approved these changes Feb 6, 2025
@etrepum
Copy link
Collaborator Author

etrepum commented Feb 6, 2025

Will merge this after v0.24.0 so there's some nightly releases before it goes live

@etrepum
Copy link
Collaborator Author

etrepum commented Feb 8, 2025

Latest couple commits:

  • Resolve conflict with main
  • Added more docs, and copied and pasted the examples from the docs to find a small mistake or two
  • Added flow types for the new stuff
  • Refactored StepwiseIteratorConfig to use a hasNext guard instead of stop so the types didn't have to lie sometimes (this is one of the things that flow does better than typescript with implies)

Should be good to merge once re-approved

@etrepum etrepum added this pull request to the merge queue Feb 8, 2025
Merged via the queue into facebook:main with commit c82e7bb Feb 8, 2025
39 checks passed
@etrepum etrepum deleted the lexical-caret branch February 8, 2025 17:40
@etrepum etrepum mentioned this pull request Feb 20, 2025
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. extended-tests Run extended e2e tests on a PR
Projects
None yet
3 participants