Description
The Problems
In #1262 (comment) I explained a series of problems when it comes to extending nodes.
At that time I thought that the best way to solve it was to group different nodes in the same class (as is done now with the different levels of Heading). In fact later in the same thread, I offered a proof of concept. I still think this could potentially reduce code repetition in very similar nodes like headings and paragraphs, but after thinking about it a lot I came to the conclusion that it is not a viable solution, at least for all the problems that I described.
After that the node replacement API came out. Although it solves some problems, I think the first 3 items of my initial post are still valid:
- When converting one elementNode to another, the shared properties are lost (for example, convert an indented paragraph to a heading and the indentation will be lost). The reason for this is that since they are different classes, a new instance is required. Of course, by changing the type of a node one could also copy the desired properties to the new node. EDIT: It seems this has been fixed only for the indent case, and not for arbitrary user-defined properties.
- Even if some properties are used only by some nodes, it would be ideal if the other nodes keep that information. The reason for this is that users often undo operations using the reverse operation instead of ctrl + z. For example, ListItem currently ignores the __indent property because it handles it with nesting. But if I have indented paragraphs, I would like the indents to be preserved when I convert them to a list and return them to paragraphs. Or if I have a checklist checked, convert it to something else, and return to the checklist, the "completed" property should be preserved. I recommend seeing this notion blog article where they talk about this.
- If the different types of elementNodes are subclassed and we have a shared property, then the same display logic must be defined in the updateDOM and createDOM of each node, which makes it very difficult to maintain the code. In Lexical right now there is a workaround with the __indent case that is handled in Reconcilier.ts. Besides being a bit confusing, the rest of us can't do the same for other shared properties we might want to add.
Things I've Tried
So once I had the node replacements API available, I decided to ignore issues 1 and 2 for the moment and focus on issue 3. That is, I tried to reduce the excessive code repetition required to add a shared property to multiple nodes at once; and I did it using a combination of the node replacement API and the mixin pattern.
In the process I ran into many difficulties, but I came up with an idea that I think would solve all 3 problems at once.
New idea: how I think extending a node should look like.
As I mentioned in the other thread, note for example how applications like Notion, Dynalist, Remnote, Roam Research and Workflowy have properties such as color shared by what in Lexical we would call nodes of type element.
If a user wanted to do something like this with Lexical, for each of the nodes they would do something like this:
export type SerializedColoredTextNode = Spread<
{
color: string;
type: 'heading';
version: 1;
},
SerializedTextNode
>;
export class ColoredNode extends TextNode {
__color: string;
constructor(text: string, color?: string, key?: NodeKey): void {
super(text, key);
this.__color = color || "defaultColor";
}
static getType(): string {
return 'colored';
}
setColor(color: string) {
const self = this.getWritable();
self.__color = color;
}
getColor(): string {
const self = this.getLatest();
return self.__color;
}
static clone(node: ColoredNode): ColoredNode {
return new ColoredNode(node.__text, node.__color, node.__key);
}
createDOM(config: EditorConfig): HTMLElement {
const element = super.createDOM(config);
element.style.color = this.__color || "defaultColor";
return element;
}
updateDOM(prevNode: ColoredNode, dom: HTMLElement, config: EditorConfig): boolean {
const isUpdated = super.updateDOM(prevNode, dom, config);
if (prevNode.__color !== this.__color) {
dom.style.color = this.__color;
}
return isUpdated;
}
static importJSON(serializedNode: SerializedMentionNode): MentionNode {
const node = new ColoredNode(serializedNode.text, node.serializedNode.color);
node.setFormat(serializedNode.format);
node.setDetail(serializedNode.detail);
node.setMode(serializedNode.mode);
node.setStyle(serializedNode.style);
return node;
}
exportJSON(): SerializedHeadingNode {
return {
...super.exportJSON(),
color: this.getColor()
tag: this.getTag(),
type: 'colored',
version: 1,
};
}
}Instead, the solution I have in mind would reduce it to just this:
export class ColoredNode extends TextNode {
static getType(): string {
return 'colored';
}
createDOM(config: EditorConfig): HTMLElement {
const element = super.createDOM(config);
element.style.color = this.getProperty(color) || 'defaultColor';
return element;
}
updateDOM(prevNode: ColoredNode, dom: HTMLElement, config: EditorConfig): boolean {
const isUpdated = super.updateDOM(prevNode, dom, config);
if (prevNode.getProperty(color) !== this.getProperty(color) {
dom.style.color = this.getProperty(color);
}
return isUpdated;
}
}Note: I think there are ways to simplify this even further, but I'll leave it as a starting point for now.
Implementation
The way to avoid defining properties, getters and setters in each class would be with the following primitives:
class LexicalNode {
//...
__properties: object;
getProperties(): object {
const self = this.getLatest();
return self.__properties;
}
setProperties(properties: object): object {
const self = this.getWritable();
self.__properties = properties;
}
getProperty(key: string): any {
const self = this.getLatest();
return self.__properties[key];
}
setProperty(key: string, value: any): void {
const self = this.getWritable();
self.__properties[key] = value;
}
}Cloning or serializing a class instance is tricky. But by making the properties a simple object we could do without explicitly defining the clone(), importJSON or exportJSON() methods for every subclass.
Each class would inherit those methods something like:
exportJSON() {
return {
//...
properties: this.getProperties(),
type: getType(),
};
}
importJSON(serializedNode) {
const node = $createXnode();
//...
node.setProperties(serializedNode.properties);
return node;
}
clone(node: XNode): XNode {
return new XNode(node.__properties, node.__key);
}Of course, an interface could be defined to the __properties object, to get the typesafety.
The point is that the properties would be decoupled from the nodes. In other words: although li, or paragraph are not going to use the "checked" property, it would still be nice if they did, which is what I explained in points 1 and 2 at the beginning of the post.
Then, keeping any arbitrary properties when converting one node to another would be trivial. You would only have to clone the properties in the $setBlocksType function
Activity