Deprecate `splice`, `append`, `getChildIndex` and `getIndexWithinParent`
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 23.9k
- Forks
- 2.2k
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 61
Description
Lexical previously used arrays to describe the children of an ElementNode but switched to double linked list for performance reasons (source).
However, the benefits have not yet been fully realized due to some reminiscences of the previous structure. The main example is the ElementNode.splice method which is clearly inspired by Array.splice. By extension, the same applies to append which is based on splice. There is also ElementNode.getChildIndex and LexicalNode.getIndexWithinParent which are O(n) and are only used for later splice. If you put O(n) in the search engine, you will find 3 comments TODO: this is O(n), can we improve?. All 3 have to do with indexes.
I propose to deprecate splice, append, getChildIndex, and getIndexWithinParent methods in favor of:
LexicalNode.insertRangeAfter(firstToInsert: LexicalNode, lastToInsert?: LexicalNode).ElementNode.appendRange(firstToInsert: LexicalNode, lastToInsert?: LexicalNode).LexicalNode.replaceRange(firstToReplace: LexicalNode, lastToReplace?: LexicalNode).
In all three cases, the methods must throw an error if the second parameter does not happen to be a later sibling of the first.
If the second parameter is undefined, the last sibling of the first parameter will be defaulted.
I already made an initial version of insertRangeAfter in #5201 because it also has the important application to modify the pasting behavior.
Question 1: what would be the best way to deprecate it? add a DEPRECATED_ prefix as was done with GridSelection, use a JSDoc comment, or remove the methods? I'm leaning towards the prefix.
Question 2: note that we would have pairs of methods: insertAfter - insertAfterRange, append - appendRange, replace - replaceRange, just as the DOM has append and appendChild.
This would not be a problem for insertAfter and replace, since neither accepts an array, but it would be a problem with append since it does accept an array of nodes. How could we call the two methods respectively? A couple of ideas:
push - pushRangeinsert - insertRange
NOTE: I heavily modified the first version of this issue so that it does not remain eternally open due to its wide scope. Once this task is finished, we could consider auditing all methods or functions that use a node array and analyze if there are opportunities for improvement.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the ElementNode and LexicalNode methods named in the issue, then read the initial insertRangeAfter implementation in #5201. Determine the deprecation strategy, final paired method names, and sibling-validation behavior; done means the agreed range APIs and deprecations are implemented consistently.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- api
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100