[lexical] Feature: add a generic state property to all nodes

[GermanJablo]

This PR enables adding arbitrary state to nodes with automatic JSON serialization.

Motivation:

1. Easier node customization

The following compares adding a 'color' property to a text node using the current node replacement API versus the state API in this PR.

With node replacement API

// IMPLEMENTATION
export type SerializedColoredTextNode = Spread<
{
  color: string;
  type: 'colored';
  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,
  };
}
}

export const nodes = [
  ColoredNode,
  {
    replace: TextNode,
    with: (node: TextNode) => new ColoredNode(node.__text),
    withKlass: ColoredNode,
  },
  // ...
]

// USAGE
const textNode = $createTextNode();
textNode.setColor("blue");
const textColor = textNode.getColor() // "blue"

With state API

// IMPLEMENTATION
const color = createStateKey('color', {
  parse: (value: unknown) => (typeof value === 'string' ? value : undefined),
});

// USAGE
const textNode = new TextNode();
textNode.setState(color, "blue");
const textColor = textNode.getState(color) // "blue"

2. Composability

If two users replace the same node with the node replacement API, the system fails.
This is particularly relevant when you do not fully control the codebase. For example, when installing a Lexical plugin that requires a custom node.

3. Document metadata

Sometimes it can be useful to store some metadata about the document, such as its ID or creation date.
It can also sometimes be useful to store maps for faster lookups. For example, the comments plugin could have a map of comment IDs to node keys.
This API allows storing that information in RootNode.

---

Notes:

• This PR is a new iteration of [lexical][@lexical/selection] Feature: add a generic classes property to all nodes for easier customization #6929 but less opinionated and with a more limited scope.
• This API does not come with opinions on how to render the state or serialize it to HTML. Therefore, the example comparison with node replacement API from section 1 is not entirely fair right now since it does not include create/updateDOM. However, the plan is to soon provide a plugin to handle this.
• The API design is the brainchild of @etrepum. Credit to him for also guiding the development.

[vercel]

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	Jan 31, 2025 6:56pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 31, 2025 6:56pm

[GermanJablo]

If I may, I would like to propose this form of opt-out which has legitimate uses. For example, in LexicalNode.test.ts when I define type variables just for the purpose of testing the types.
Also, while developing a WIP it's convenient to be able to mute warnings without having to write comments everywhere.

[etrepum]

I prefer this setting also, but maybe there’s some meta style guide reason for not having this? /cc @zurfyx

[GermanJablo]

Perhaps I could have used Object.defineProperty to avoid having to modify the tests, but I preferred to do this because it turned out that there were few tests affected and the changes were trivial.

[etrepum]

I think this will probably be necessary to implement for correct yjs behavior

[GermanJablo]

I already followed your suggestion to copy the state object in setState, so this suggestion is no longer necessary?
On the other hand, do you know what a unit test for yjs could look like?

[etrepum]

I am not familiar with the intricacies of testing lexical-yjs off-hand, I just know its implementation is very problematic because it manipulates node properties directly rather than going through the constructor and well defined serializations. I'd find another node that uses a non-primitive property (something to do with comments or marks probably?) and see if there's any relevant testing. There might not be any good testing for this sort of thing currently.

[GermanJablo]

I tried to see if collision checking could be done at runtime, but I came to the conclusion that it's not a good idea.

• Key registration would have to be global
• It would be difficult to differentiate it from re-executions of the same definition (e.g. useEffect)
• it might even be better to leave the conflict silent than crash the program. Two plugins using bg-color would probably do the same thing.

In any case, the recommendation here in the documentation would still be valid.

[etrepum]

A map+object API would allow this kind of check on first use although to make this efficient you’d use Map<string, StateKey> and have the object be underlying storage (or have a separate object for parsed values)

[GermanJablo]

I don't know how you guys handle redirects on the website, but I think it would be nice to rename the url to this title.

[GermanJablo]

As I said about collisions above, I think having a global registry would make things more complex. An alternative I can think of is using setState to remove the default value:

  setState<T extends StateKey>(k: T, v: T['value']) {
    const self = this.getWritable();
+   if (k.parse(undefined) === v) {
+     delete (self.__state as State)[k.key];
+   }
    (self.__state as State)[k.key] = v;
  }

[etrepum]

The map+object implementation that I described in the chat would resolve this since the keys of the map are state keys and not strings

[github-actions]

size-limit report 📦

Path	Size
lexical - cjs	29.17 KB (+0.34% 🔺)
lexical - esm	28.98 KB (+0.41% 🔺)
@lexical/rich-text - cjs	38.18 KB (+0.16% 🔺)
@lexical/rich-text - esm	31.02 KB (+0.35% 🔺)
@lexical/plain-text - cjs	36.68 KB (+0.3% 🔺)
@lexical/plain-text - esm	28.29 KB (+0.25% 🔺)
@lexical/react - cjs	40.05 KB (+0.37% 🔺)
@lexical/react - esm	32.46 KB (+0.56% 🔺)

[etrepum]

The map+object implementation that I described in the chat would resolve this since the keys of the map are state keys and not strings

[etrepum]

A map+object API would allow this kind of check on first use although to make this efficient you’d use Map<string, StateKey> and have the object be underlying storage (or have a separate object for parsed values)

[etrepum]

I think this will probably be necessary to implement for correct yjs behavior

[etrepum]

With a single object implementation this must always call parse because we don’t know if its directly from JSON or not

[etrepum]

This logic also needs to go into afterCloneFrom, and probably worth adding tests to make sure that this feature doesn’t rewrite history (setting state shouldn’t affect previous reconciled versions of the node)

[GermanJablo]

Any idea how to test it? I tried comparing _editorState with _pendingEditorState without success

[etrepum]

let v0: RootNode;
let v1: RootNode;
const vk = createStateKey('vk', { parse: (v) => typeof v === 'number' ? v : null });
editor.update(
  () => {
    v0 = $getRoot();
    v0.setState(vk, 0);
    expect(v0.getState(vk)).toBe(0);
  },
  {discrete: true},
);
const state0 = editor.getEditorState();
editor.update(
  () => {
    v0.setState(vk, 1);
    // normally would use chaining for this but setState doesn't return this
    v1 = v0.getLatest();
    // This is testing getLatest()
    expect(v0.getState(vk)).toBe(1);
    expect(v1.getState(vk)).toBe(1);
    expect(v1.is(v0)).toBe(true);
  },
  {discrete: true},
);
const state1 = editor.getEditorState();
// Test that the correct version is returned and that they are independent
expect(state0.read(() => v0.getState(vk))).toBe(0);
expect(state1.read(() => v1.getState(vk))).toBe(1);
// Test that getLatest is used and not the __state property directly
expect(state0.read(() => v1.getState(vk))).toBe(0);
expect(state1.read(() => v0.getState(vk))).toBe(1);

[etrepum]

I am not familiar with the intricacies of testing lexical-yjs off-hand, I just know its implementation is very problematic because it manipulates node properties directly rather than going through the constructor and well defined serializations. I'd find another node that uses a non-primitive property (something to do with comments or marks probably?) and see if there's any relevant testing. There might not be any good testing for this sort of thing currently.

[etrepum]

This type is a lie because it will return undefined and call it a string. Not sure what the point of an intentionally wrong cast here is?

[GermanJablo]

yeah, I wanted to test the type (line 1539). What you say I'm doing in 1544 to 1551. Here I could throw an error if value is not typeof string, but for the purposes of this test it doesn't matter.

[etrepum]

Yeah but there's no good reason to lie here because we're not testing a runtime invariant. At least (value as string) || '' or something like that would make more sense. Sometimes people look at the tests to see how to use something and this is just lying for no good reason because undefined is always an inhabitant.

[etrepum]

I don't really understand your comment here, there's no error because you can explicitly cast unknown to anything that you like without any sort of warning or error in typescript. The only difference from any is that it requires an explicit "trust me bro". It's perfectly valid to have a function with this type signature, it's just the implementation that's wrong.

Suggested change

	// TO-DO:
	// It would be a bit nicer if createStateKey also gave a compilation error if parse tried to return null.
	// It's not a big deal, because getState does give an error. The problem is that I can't activate it.
	// The @expect-error does not work because strict mode is disabled in `tsconfig.test.json`.
	const _keyForMaybeNull = createStateKey('keyForMaybeNull', {
	parse: (value) => value as string | null,
	});
	// // @ts-expect-error - null is not a valid value
	// const _maybeNullValue = root.getState(keyForMaybeNull);
	const keyForMaybeNull = createStateKey('keyForMaybeNull', {
	parse: (value) => typeof value === 'string' ? value: null,
	});
	const maybeNullValue = root.getState(keyForMaybeNull);
	expect(maybeNullValue).toBe(null);

[GermanJablo]

If you look at the State type, I specifically made sure it doesn't allow null. I find it better to incentivize undefined for smaller serialization. If you uncomment these lines, you can see that tsc gives an error in the editor because it uses tsconfig.json, but not in CI because it uses tsconfig.test.json

[etrepum]

Your intent to restrict the type doesn't really provide any benefit, given parse: (value) => typeof value === 'string' ? value : null then null will never be serialized if the implementation follows the guidelines that I proposed.

There are use cases where null is an explicit value that's different from the default (which could be undefined, or any other value), and your proposed type makes that use case impossible.

[etrepum]

For more clarity here is a tsplayground demonstrating how I think this should all work https://tinyurl.com/rv8jutvt

code below for posterity

import { z } from 'zod';
import { Schema, Option } from 'effect';

export type DeepImmutable<T> = T extends Map<infer K, infer V>
  ? ReadonlyMap<DeepImmutable<K>, DeepImmutable<V>>
  : T extends Set<infer S>
  ? ReadonlySet<DeepImmutable<S>>
  : T extends object
  ? {
      readonly [K in keyof T]: DeepImmutable<T[K]>;
    }
  : T;
export type State = {[Key in string]?: StateValue};
// This could be relaxed in a future version, but for now we only
// support values that are directly JSON serializable
export type StateValue = string | number | boolean | null | undefined | State;
export interface StateKey<
  K extends string = string,
  V extends StateValue = StateValue,
> extends StateKeyConfig<V> {
  readonly key: K;
  // Here we are storing a default for convenience
  readonly value: DeepImmutable<V>;
}
export interface StateKeyConfig<V extends StateValue = StateValue> {
  readonly parse: (value?: unknown) => V;
}
export type StateKeyConfigValue<T extends StateKeyConfig> = ReturnType<T['parse']>;

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function createStateKey<K extends string, T extends StateKeyConfig<any>>(
  key: K,
  config: T,
): StateKey<K, StateKeyConfigValue<T>> {
  return {key, value: config.parse(undefined), parse: config.parse.bind(config)};
}

// It's best for us to demonstrate actual parsers, but we probably
// shouldn't ship a parse library since many users will already have
// something (zod, effect, whatever)
function parseString<Default extends StateValue>(defaultValue: Default): StateKeyConfig<string | Default> {
  return { parse: (v: unknown) => typeof v === 'string' ? v : defaultValue };
}
const skNull = createStateKey("test", parseString(null));
//    ^?
const skUndefined = createStateKey("test", parseString(undefined));
//    ^?
const skString = createStateKey("test", parseString(''));
//    ^?

const zkNull = createStateKey("test", z.string().nullable().catch(null));
//    ^?
const zkUndefined = createStateKey("test", z.string().optional().catch(undefined));
//    ^?
const zkString = createStateKey("test", z.string().catch(''));
//    ^?

function parseSchema<A extends StateValue, I, Default extends StateValue>(schema: Schema.Schema<A, I, never>, defaultValue: Default): StateKeyConfig<A | Default> {
  return { parse: (value) => Option.getOrElse(Schema.decodeUnknownOption(schema)(value), () => defaultValue) };
}

const ekNull = createStateKey("test", parseSchema(Schema.String, null));
//    ^?
const ekUndefined = createStateKey("test", parseSchema(Schema.String, undefined));
//    ^?
const ekString = createStateKey("test", parseSchema(Schema.String, ''));
//    ^?

[etrepum]

null should be a valid inhabitant of all of these types

[GermanJablo]

I find it better to incentivize undefined for smaller serialization.

[etrepum]

Not necessary. When null is the default then it won't be serialized. If null is not the default then it's an explicit choice to include null and there's no good reason to prevent that.