facebook / lexical

Lexical is an extensible text editor framework that provides excellent reliability, accessibility and performance.
https://lexical.dev
MIT License
18.2k stars 1.52k forks source link

Feature: Provide a variant of EditorState.read that sets activeEditor #6346

Open etrepum opened 5 days ago

etrepum commented 5 days ago

Description

EditorState.read can only use $functions that don't depend on activeEditor which ends up leading users to use Editor.update even for fundamentally read-only operations. A common use case would be $getNodeFromDOMNode or $getNodeFromDOM or to avoid having explicit LexicalEditor parameters for $functions ($generateHtmlFromNodes, $getHtmlContent, etc.).

We should have a way to set the activeEditor with an additional argument to EditorState.read.

There should also be a version of read directly from the LexicalEditor for symmetry with update. This would also provide an opportunity to optionally get the _pendingEditorState so that users can "read their own writes" from an update before reconciliation has occurred.

The public API changes would be as follows:

export interface EditorReadOptions {
  // Not sure whether the best default would be to
  // use pending or the latest reconciled state
  pending?: boolean;
}

class LexicalEditor {
  // …
  read<V>(callbackFn: () => V, options: EditorReadOptions = {): V {
    const editorState = (options.pending && this._pendingEditorState) || this.getEditorState();
    return editorState.read(callbackFn, { activeEditor: this });
  }
}
export interface EditorStateReadOptions {
  editor?: LexicalEditor | null;
}

class EditorState {
  // …
  read<V>(callbackFn: () => V, options?: EditorStateReadOptions): V {
    // Changes would be required to internal readEditorState to support this
    // Using editor as first argument for symmetry with lexical-devtools lexicalForExtension internal API
    return readEditorState(options && options.editor || null, this, callbackFn);
  }
}

Impact

Users would have a simpler to explain model for what a $function can do, as most would transition from using editor.update for read-only operations to editor.read and people will also likely prefer editor.read to editor.getEditorState().read.

The extra argument to EditorState.read would mostly be an implementation detail that I don't think most people would use, but it keeps the code path very similar to the current paradigm.

fantactuka commented 5 days ago

Not sure whether the best default would be to use pending or the latest reconciled state

I think it would be easier to reason about this API if it's an alias to

editor.getEditorState().read(() => {}, { activeEditor: editor })

and uses latest reconciled state.

zurfyx commented 4 days ago

I think it would be easier to reason about this API if it's an alias to

editor.getEditorState().read(() => {}, { activeEditor: editor })

@fantactuka This assumes that the editor is compatible with the EditorState and makes the EditorState from getEditorState redundant. Is this correct? In this case, I think that editor.read depicts more the point of let's try read the most up to date state we can find on the editor

etrepum commented 4 days ago

In #6347 I implemented the version where the reconciled state is the default and pending state can be acquired with an option.

Reasons why you might want to read from reconciled state:

Reasons why you might want to read from pending state:

An alternative to this "pending" approach would be a flushSync or discrete option (or a separate readSync method) that would force a reconciliation before the read (if necessary) which would give you a consistent view after transforms and DOM reconciliation.