Coder Social home page Coder Social logo

Comments (10)

acywatson avatar acywatson commented on June 14, 2024 3

You don't install a package to use the library in headless mode. On the contrary, you install a package to be able to use it in the DOM. I always found it strange that Lexical does the other way around. I see that one of the goals of this RFC is to minimize the bundle size and responsibility of the core, and I think making sure that the core works without the DOM could be another strategy to optimize this.

This is a good insight

from lexical.

acywatson avatar acywatson commented on June 14, 2024 1

Nice write up. Some initial thoughts:

Since Lexical inception we have avoided having multiple routes to solving the same goal. There is often a better way and this can easily lead to inefficiences.

This is just an opinion, but I don't think your examples really prove that there are significant inefficiencies here. I also think you discount the advantages of having multiple routes to the same goal - usually the other routes exist because ergonomics dictate that they should.

To create an S3 bucket, you can use the CLI, SDK, or Console. You can use S3 directly, you can use CloudFormation, or you can use the CDK to generate a CloudFormation stack.

All of these make sense for different people with different use cases.

I actually don't think this tenet has as much bearing on your argument/proposal as you suggest, but this is something that I'd push back on as a matter of principle.

Static transforms. We don't want users to build a full-fledged plugin inside the Node itself. It is not feasible to build most plugins this way, for example, plugins that have UI attached like CharacterLimit or plugins that depend on multiple nodes like Hashtag. This wildcard transform function inside the Node causes unnecessary overhead for developers to find where the code lives. We can improve the UX with a simple tweak (see Normalizer section).

I agree with your conclusion, I think, but how you got there doesn't make much sense to me. I don't understand the connection between the static transform API and "a full-fledged plugin", or even why a full-fledged plugin would be bad. The intention of this API was never to allow building plugin-like behavior on nodes (and AFAICT, it hasn't?). It's a simple ergonomic tweak - you literally have to pass a node in when you register a transform. So putting the transform on the node is just a declarative way of achieving the same thing.

Nodes have to be loaded before we can render the EditorState but a method like remove doesn't play any role in the bootstrap.

this is a good point. messy to implement. My only concern would be whether the juice is worth the squeeze.

These will be imported on createEditor, just like normalizers. This reinforces the modularity aspect of Lexical and makes it possible to have a much lighter version of a plain text editor. #6020

This API already exists, right? #4254

from lexical.

GermanJablo avatar GermanJablo commented on June 14, 2024 1

I think this is reasonable, but with RangeSelection it will be hard to make it an independently module. Functions like remove or replace recompute the selection in arbitrary places. @zurfyx

I think the heuristics with selection happen when trying to delete nodes. An error might be thrown that says something like: "You cannot delete nodes in an environment without selection."


I think this RFC is very broad and continues to branch. It's hard for me to follow the thread and I think there may be interesting points left behind. I suggest that it be divided into the following:

A. Discussions that have to do with decoupling parts of Lexical to improve bundle size or performance in certain scenarios:

  1. separate read and write properties of a node.
  2. centralize HTML serialization and perhaps createDOM outside of nodes
  3. no headless package, DOM as an extra module.
  4. decouple the selection from the editor.

B. Discussions that do NOT have to do with decoupling parts of Lexical to improve bundle size or performance in certain scenarios:

  1. RegisterPlugin
  2. Normalizers

If there is support, we can open these issues and copy the relevant information from this thread. If there is any idea or proposal that I have overlooked, please tell me.


Regarding the points that are in category A, I think their viability will depend on the complexity they imply. For example, if (A.1) involves having a class ParagrahNode_read and ParagraphNode_write, I don't know if the cost outweighs the benefit.

from lexical.

GermanJablo avatar GermanJablo commented on June 14, 2024
  • I understand that registerPlugin would be similar to mergeRegister only that it would make debugging easier. I like it.
  • About "decouple the read and edit functionality of the Nodes". I understand the benefit, although I am concerned about the complexity involved. Since the day Valibot came out I have given this matter quite a bit of thought. Valibot is basically Zod, only instead of using a large class, it uses a function for each method, allowing for better tree shaking. This seems insane to me. Something that I have thought since then is that programmers should not choose between FP and OOP thinking about the bundle size, but that the optimization should be handled automatically by tools like bundlers. Sorry if I'm taking this too theoretically, just a thought I need to share. As you said, "This is worth its own issue."
  • Normalizer: taking your words that you want to "avoid multiple routes to solve the same objective", I do not understand what problem normalizers solve that do not already solve transforms as has been exemplified in the last comments of #3833. Maybe I'm missing something, but if not, I'm leaning toward not adding a new API.
  • About moving HTML serialization to editorConfig, it seems like a good idea to me.

In addition to the points mentioned, I want to suggest a couple more that I think follow the same purpose of this RFC.
In this last month, I have been working on a spreadsheet library. I've gotten a lot of ideas from Lexical and the API is in many ways similar. However, I have made 2 decisions that I believe are in line with the reconsiderations of responsibility that are being made here:

  1. Selection and the rest of the state are independent: The Editor class (grid in my case) might not have any knowledge of the Selection class. In some environments such as headless editor, selection is not necessary, and as with the editing methods of a node, many unnecessary lines of code are used. I think this decoupling can be achieved without breaking changes, since the selection is accessed with $getSelection() instead of editor._selection.
  2. You don't install a package to use the library in headless mode. On the contrary, you install a package to be able to use it in the DOM. I always found it strange that Lexical does the other way around. I see that one of the goals of this RFC is to minimize the bundle size and responsibility of the core, and I think making sure that the core works without the DOM could be another strategy to optimize this.

from lexical.

etrepum avatar etrepum commented on June 14, 2024

My personal preference for this kind of thing is to allow "power users" the ability to achieve their goals but in all other cases convenience/usability "in the small" should be the preferred mode. I think in most cases people are not trying to implement every node twice, or in multiple pieces, to support readonly and editable nodes in order to optimize runtime overhead. Should it be possible? Sure. Should the developer have to manage it at the LexicalNode subclass layer? Probably not. If others agreed this was the approach to take I would also recommend other changes to the API, mostly removing "obvious required defaults" like namespace and onError in LexicalComposer, ErrorBoundary and ContentEditable in RichTextPlugin, or shouldBootstrap in LexicalCollaborationPlugin.

I think at this point we have the capability to present a better model. I personally prefer to keep code together unless there's a really good reason to split it up, and in most cases people have one editable use case and those transforms/schemas/normalizations/whatever are all very intrinsically linked to the node so it feels awkward to be required to have to implement some other thing for that node to function correctly (especially in a react-less use case). With this angle I think it would make sense for a Node to have a way to register itself with the editor to provide whatever necessary listeners/transforms/importers/etc. it has. Maybe this can be split up in to some sort of handshake so that the DOM or editable stuff can be split into a separate async loaded chunk if the editor does not request it.

The simplest way as-is is to just say to power users, well, if you want this complicated thing so badly, why not just register different nodes for the same type across read-only and editable use cases. Everything is already ready for that because the type of the node is not associated with the identity of its constructor (and even if it was, there's the node replacement escape hatch). The editable version of the node can subclass the read-only version of the node to inherit the functionality without burdening the read-only use case with the editable code.

Another solution of course is to not care about any of this and do some form of SSR such that only the output of the Lexical code is running on the client in a read-only use case.

My only concern about moving HTML serialization to editorConfig, at least in a separate way from the array of nodes to register, is adding more boilerplate for the users to write or to make mistakes if it's not computed. I'm all for having a way to do overrides, I just think the boilerplate for obvious behavior should be nonexistent whenever possible.

from lexical.

etrepum avatar etrepum commented on June 14, 2024

Maybe a lot of these things could be solved if the Node was able to vend "static async" methods for configuring these things that could load the required code on demand (for bundle size optimization use cases)? And the editorConfig would have something like node replacers where you could use a node but provide your own versions of those static async methods (which could of course easily call out to the original implementations if you're extending rather than completely overriding)

from lexical.

abelsj60 avatar abelsj60 commented on June 14, 2024

Hi Gerard. Long time. I'm out of date and behind the times, but since you pinged, I'll try to offer a few comments.

While I tend to agree with Acy on this — strongly in fact — I think you're a clever guy who steeps himself in his projects.

For instance:

  • I knew Lexical needed a dedicated, first-class TabNode
  • I knew after twelve and a half seconds, I wasn't the guy to build it
  • I knew I'd never be able to marshal the kind of argument needed convince you all of it

You were the guy who looked at the issue at some point and realized that Lexical needed a dedicated, first-class TabNode, then spent a lot of time getting it to work right because it turned out to be quite a lot more complicated than it seemed.

Given that, I'm sure you're responding to a lot of real world intuitions from day-to-day work on the project.

(Yeah, I'm an odd duck, I think intuition is generally more valuable than "data." All day long. /shrug)

I'm not sure neutering Lexical's flexibility is the way to go about addressing that, though.

Maybe there's another way to explain and address the issues you're sensing in your work? Personally, I'm reading the bundle size concerns more as an attempt to make a persuasive argument than a big current concern that keeps you up at night.

Generally:

The Lexical Way

Some of what you say here makes a bit of a tangential point that may be relevant. For instance:

Nodes used to represent the data (we intentionally excluded any reference to editor) and wrap simple configuration that either events and reconciler needed to know, but now they are responsible for too much to the point that you can build a full-fledged plugin in the Node itself.

Is this something new users — or even advanced ones (if advanced means knowing a lot of the API) — understand? I know there have been attempts to improve the docs, but I wonder if people just don't get "the Lexical way."

Generally, people have a paying job to do and reach for whatever works fastest.

That Lexical can offer so many ways to help them is a great strength.

But, maybe you could help them find what works faster if you could better communicate the Lexical ways to them.

I don't mean the API — I mean how to think about Lex, which is really a one-for-one DOM swap.

RSC and SSR

  • Will Lexical work with React Server Components?
  • Would the question about SSR above make it more likely that a Lexical read-only editor for presentation can be built and delivered correctly without any headaches?
  • In the past, people asked for help with SSR because they were struggling with it. Given where Remix is, and Next, which I avoid, maybe this general question is a primary issue today that should be handled before making any broader decisions about the model? (Unless it already is? I've not been in Lex lately.)

Schemas OR non-linear node assembly

The last time I commented on the schema issue, my concern was Lexical objects that are not built in a linear fashion — stuff with children and grandchildren. It's hard to do.

I did it with LinedCodeNode. You have access to it in Lexical 401.

The code won't impress you, but it used to work and make the point. Perhaps you at FB deal with broken editor states a lot, so fixing them with a schema is important.

But, I struggled most with Lexical in the context of a node that oversaw multiple other nodes.

(I did a lot of that. I have things called LexicalInput, LexicalSelect, and LexicalCommandBar, for instance.)

I remember the table and experimental table were similarly complicated to understand in terms of createDom, updateDom, and the import/exports. I don't remember the ins-and-outs today, but one issue was you could create/update and import/export from the top level node or a child and there was no guidance beyond Dominic's use of the top-level in his tables.

Perhaps if you looked at this type of issue — including the tables — and considered an easier method of handling it all, you would get some ideas for changing the API in a way that would address your underlying concerns here. These strike me as among the most complicated uses of Lex, so making them easy may make a lot of things easy.

(You could even build an official LinedCodeNode, I remember Dom didn't love the current CodeNode. I'd agree.)

--
That's all I've got. Probably what I've said before... I'm behind, so maybe you, Acy, and co have already addressed all this, but I'd guess there's something under the surface here that's bothering you.

I'm not sure I like the idea of "simplifying" it the way described, and I tend to think Dominic was right to try to keep data flowing one way with plugins. But I can't restate why the way he did. Maybe that's the problem here — my inability to articulate this type of thing demonstrates why people aren't following the Lexical way as well as they could?

It's not the API they struggle with so much as understanding how to put it all together.

Generally, I think Lex is a great project, particularly since it exists to literally react to language. Given that AI is now language driven, and clearly going to swallow most of our machine interactions, what could be more important?

from lexical.

zurfyx avatar zurfyx commented on June 14, 2024

Selection and the rest of the state are independent: The Editor class (grid in my case) might not have any knowledge of the Selection class. In some environments such as headless editor, selection is not necessary @GermanJablo

I think this is reasonable, but with RangeSelection it will be hard to make it an independently module. Functions like remove or replace recompute the selection in arbitrary places.

Second, I think it'd make sense to pair this with some new listeners, registerSelectionListener and registerEditorStateListener, to convey the fact that these are separate concepts. The History plugin would also need to be updated accordingly.

This one deserves its own issue with a proper API and trade-offs evaluation.

I remember the table and experimental table were similarly complicated to understand in terms of createDom, updateDom, and the import/exports. @abelsj60

That's true, @GermanJablo did some good work on the API simplification but these methods you pointed out have never been revisited, updateDOM is particularly hard to understand.

convenience/usability "in the small" should be the preferred mode. I think in most cases people are not trying to implement every node twice, or in multiple pieces, to support readonly and editable nodes in order to optimize runtime overhead. Should it be possible? Sure. Should the developer have to manage it at the LexicalNode subclass layer? Probably not. If others agreed this was the approach to take I would also recommend other changes to the API, mostly removing "obvious required defaults" like namespace and onError in LexicalComposer, ErrorBoundary and ContentEditable in RichTextPlugin, or shouldBootstrap in LexicalCollaborationPlugin. @etrepum

I support the simplicity aspect for new users (and so does @abelsj60 based on the coment above) but I would be keen to explore options that didn't sacrifize power users. We can add code but we can't remove it, the last code example on the builder I provided shows a potential way to achieve so, by providing a very flexible lower level layer but promoting the abstraction which is easier to set up.

One of the conflicts we have faced between Meta, Bloomberg and other open-source folks utilizing Lexical is the number of features supported in TextNode importDOM. For example, at Meta, we don't use colors, whereas other companies would like this feature supported. Overriding the HTML, the API @acywatson created, unblocks this issue but the 200 LoC in importDOM still remain there.

At the same time, we have always supported modularity on Lexical. Hence, why we have 24 packages under @lexical, but we can provide bundles so that it's easier to spin up a rich text editor. In fact, we do this internally already, where our simplest API looks like <XYZEditor value={initialValue} onChange={callback} label={a11yLabel} toolName={forLoggingPurposes} />.

Maybe a lot of these things could be solved if the Node was able to vend "static async" methods for configuring these things that could load the required code on demand (for bundle size optimization use cases)? @etrepum

I'd be keen to read some code example on this idea and the potential implications on the bundling.

from lexical.

fantactuka avatar fantactuka commented on June 14, 2024

Normalizers never run on the bootstrap update nor in non-editable mode.

Agree with non-editable mode, but wonder if normalizers on bootstrap would be useful when new normalization rules are introduced or content was corrupted before normalizers were introduced. Although it should be easy to resolve with some sort of plugin that force-updates on bootstrap that would trigger normalizers

Read-only nodes

At some point we had an idea of (internal) gencode that would generate nodes for headless mode (running on HaaS) vs regular ones. We wanted it for different reasons, but separate read/write nodes could be a way to trim down bundle for read-only mode in case it's an actual problem for a project. I might be wrong about users' expectations, but if I had simple rich text editor (no fancy decorators, except maybe images), I'd traverse editor state to output HTML/react/whatever vs running editor instance.

Import/export DOM

An idea of co-locating it with node definition felt right at the time, although its flexibility seems to be a common problem and we tried to handle it with new concepts (IIRC, node overrides were mostly introduced to help with it) and patches, and it's still not great. With a wild west of markup that is being pasted into the editor in addition to custom nodes support and all sorts of formatting it's almost impossible to handle it all out of the box. Moving it back to editor config that might have some defaults (but the ones you can easily override) looks reasonable.

Must admit that exportDOM felt as a very natural part of the node (unlike import that had to include certain complexity to avoid conflicts with other nodes, e.g. priority), but for the sake of keeping things together it should be co-located with import

from lexical.

zurfyx avatar zurfyx commented on June 14, 2024

transforms/schemas/normalizations/whatever are all very intrinsically linked to the node @etrepum

Yes, but the ownership model is different. Methods inside the Class are created by the owner of the class, document rules do not necessarily have to adhere to this. Let's say that you want to ban the use of token TextNode inside HeadingNode, as a product owner you neither own TextNode nor HeadingNode. The options that you have are either override the Node (which is fairly convoluted) or a transform somewhere. Overriding the Node would work for this case, but it gets messy when multiple rules require overriding the nodes, particularly if one of these is part of a package you don't control.


An alternative option to keep the simplicity aspect you mentioned earlier is to expand the transforms options to fulfill normalization and promote the use of transforms immediately after editor creation, which we currently are not in LexicalComposer.

I see 4 major requirements of normalizers:

  1. They need to be registered immediately after editor creation. The first user update should already run through them all. In VanillaJS, plugins meet this expectation but the API for transforms on plugins have the implication they can be deregistered.
  2. Built-in and custom rules. Users may want to specify their own custom document rules.
  3. Simplicity: you can import the entire package dependencies with an import, just like you currently can for a Node.
  4. Optional. Normalizers attempt to fix broken document rules, in a perfect world normalizers wouldn't need to exist.

The revised EditorConfig would still retain the same API but behind the scenes, it would equal to:

createEditor({
  onError: ...,
  nodes: [
    TableNode,
    TableRowNode,
    TableCellNode,
  ],
  transforms: [
    $rowCellsNormalizer,
  ]
});

/// equivalent
createEditorBuilder({
  onError: ...,
  ...
})
  .addDependencies(TablePluginDependencies)
  .build();

from lexical.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.