Slate: introduce a "refs" concept

[ianstormtaylor]

Do you want to request a _feature_ or report a _bug_?

Idea.

What's the current behavior?

Right now the Point model in Slate has "paths" and "keys" (in addition to "offsets") to reference a location in the document. Paths are generally more performant, since you can use them to traverse directly to a node. Whereas keys need to be looked up by searching through the entire tree. (We cache keys to try to alleviate this issue, but the caches are invalidated often.)

Ideally we'd use paths in many more places, but the problem with paths is that you can't guarantee they aren't stale after an operation is applied because it may have been invalidated by insert_node/remove_node/etc. operations. The same goes for offsets as well as paths, since insert_text/remove_text operations could invalidate an offset as well.

But we might be able to create editor-specific objects to alleviate this...

What's the expected behavior?

We could create a mutable Pointer model that referenced a point in the editor's document. When you create it, you pass in the Point you're wanting to keep an up-to-date reference to.

const pointer = new Pointer(point)

Then, whenever a new operation is applied to the editor, it updates all of the pointers it's keeping track of, so that they aren't stale. And at any time in the future, you can access pointer.path or pointer.offset and it will be guaranteed not to be stale.

const { path, offset } = pointer

This could alleviate a lot of the current uses of keys. Even renderNode would be able to receive a Pointer instance since its mutable and referentially equal. (It can't receive the path right now because every time the path changed it would force a re-render.)

However, this could also help for asynchronous commands, like inserting images, where you need to perform some asynchronous work (eg. uploading the image) before you insert it, and you need to make sure that the insertion point stays updated as more editing happens.

---

Also if anyone has a better name than "pointer" I'm all ears!

[CameronAckermanSEL]

Would index be an inappropriate name? (That might create confusion with array indexes 🤔 )

[Dundercover]

What is your reasoning in keeping it mutable instead of immutable and closer to the editor? like editor.createPointer and editor.getPointerValue?

Instead of Index I would much rather see Pointer or something more verbose like PointReference or PointRef. Will there ever be higher abstractions of this like NodeRef or RangeRef?

[ianstormtaylor]

@Dundercover I like the *Ref approach to be extendable in the future if we do indeed have higher level abstractions for this kind of thing. I was thinking that a lower level PathRef would be useful in some cases too.

My reasons for keeping it mutable was so that we could create one to pass to the <Node> component, and then have it continue to be updated over time while the component maintains a reference to it, without having to re-render each time it changes? Could be a better approach here, what do you think?

As for API, maybe we can even more closely borrow from React, and similar to what you mean, we could have:

const pointRef = editor.createPointRef(point)

// A reference to an up-to-date immutable `Point` object.
pointRef.current

[Dundercover]

I like the similarities to React but will it potentially be a lot of refs? would it be able to handle a lot of refs performance wise?

[ianstormtaylor]

@Dundercover not sure, and that's a good question. It would tradeoff on read performance for key lookups for write performance updating refs. Not sure which way it would fall to be honest. But I'm curious about getting rid of the need for keys as one of the steps towards using plain objects.

[ianstormtaylor]

Another consideration is that refs could be made to be "lazy", storing the operations that have occurred, but only processing them to calculate the current value when accessed.

[Dundercover]

I think it's a great idea that we definitely should investigate. I'm uncertain about lazy evaluation, can we calculate next point with just an operation?

[Dundercover]

How should we deal with deletes? Will the PointRef always work as long as the document isn't empty?

[ianstormtaylor]

Yup, all we need is the operations that have occurred since the point was first created. And we transform with them, similar to operational transform but slightly simpler.

For deletes, the ref isn’t guaranteed to return a point. It might also be null so the user has to guard against that.

[Dundercover]

Alright, thanks for the clarification.

When talking about the <Node> use case, why do we want to keep a PointRef instead of a NodeRef or a PathRef?

[ianstormtaylor]

@Dundercover we wouldn't, we'd ideally want to keep a PathRef I think.

[zhujinxuan]

Two ideas:

1. I think it is a good idea of using pointer instead of keys, but I am a bit worrying whether we are making everything inside the Editor model. I think we can wrap every on-change things inside the editor model, but it seems a bit unnatural to wrap path inside editor as well. I think we can think about putting it into some kinds of query, then we can separate it from editor with a query interface if necessary.

2. I think we can think of the PointRef as some sort of query.

const query = editor.createTrackingQuery(block);
editor.doSomeChange(...);
const newBlock = editor.query(query);

Then we do not need to expose a new concept to users.

It means we may need a private and temporary query. I think we can make the return query as an object and use a weakmap to register the object.

The query interface is also for increment change for slate. At the very beginning, we can just replace all keys with the trackingQuery, and then we can implement the trackingQuery and then test it.

[ianstormtaylor]

@zhujinxuan I'm not sure I understand the benefits there? In your example, what are we gaining instead of just doing:

const pointRef = editor.createPointRef(point)

// A reference to an up-to-date immutable `Point` object.
pointRef.current

[zhujinxuan]

@ianstormtaylor Hi, I am thinking something like this:

We can see query and pointRef share things in common as following ways:

1. query takes an editor, and then returns a node or other result editor.query(q) => node|any
2. pointRef takes an editor, and then returns a node. editor.find(pointRef) => node
   Both are just asking for a result, and neither has side effect.

So we can think a pointRef as a kind of query. We can merge them in one interface, like

const pointRef = new PointQuery(editor, node);
editor.doSomething(...);
editor.query(pointRef);  // new node

I think the gaining is that we do not need to introduce a new model explicitly. If we think pointRef as a query, we can use query as the single source of asking values from editor.

Another problem is that pointRef.current is changed according to the editor change, which is implicit. Personally, I feel making get a mutable value (and whose mutation is changed by calling editor.doSomething rather than pointRef.doSomething) makes the pointRef.current hard to predict.
When passing an pointRef around, we do not explicitly pass the editor as another argument; it would be hard to track where editor it is when one pass pointRef to another function.

[ianstormtaylor]

@zhujinxuan that's a really great point! Thank you! I like the idea of keeping the concept complexity low. So we could instead do as you say:

const pointQuery = editor.createPointQuery(point)

// And then later...
const point = editor.query(pointQuery)

And the editor will internally keep track of things so that the query always returns the updated point/path/etc. to the caller.

[ianstormtaylor]

This is trickier than I thought, because it's hard to have the refs/queries get garbage collected. I was originally thinking of implementing this as a non-React-specific part of Slate core, which leaves us without a great place to "unref" the memory.

If it were just for React, I think we'd be able to use the lifecycle hooks to unref automatically when a component is destroyed, which prevents us from having a leak. But I'm not sure there's a way to do it (without native WeakRef support) otherwise.

Would love anyone's thoughts!

[ianstormtaylor]

Fixed by https://github.com/ianstormtaylor/slate/pull/3093.