Propose history tracers

[marijnh]

This proposal adds features to the prosemirror-history package that allow you to see when a given step is undone or redone.

An initial implementation exists in this branch.

Rendered RFC

[saranrapjs]

This would be super useful!

[saranrapjs]

In case it's informative, our team at the New York Times has already done exactly this, and it has worked well for exactly these non-mutating use-cases you describe.

[marijnh]

That's helpful to know, thanks.

[saranrapjs]

This isn't part of the scope of this RFC as it stands, but I think Tracer seems like it could also be a nice fit for the rebase operation that happens in prosemirror-collab as well; they both mainly concern themselves with steps, and both can have the effect of replaying steps that may or may not have trace-able data in them. Just a thought!

[marijnh]

Hm, that's a good point. Just to check if we're on the same page, at a minimum this would involve:

• Moving tracers into a more central package, probably prosemirror-state

• Having the collab module add tracers with a type like "rebase" when it rebases traced steps

Can you sketch a situation in which such information would be used?

[saranrapjs]

Yes....some pseudo-code for a hypothetical transaction utility function that wanted to consume/produce such things:

import { tracers } from 'prosemirror-tracer'

function wrapWithMetadata(tr, userId, isRebaseable) {
    let tracers = tr.getMeta(tracers)
    let traceData = []
    tr.steps.forEach((step, i) => {
        traceData.push(new Tracer(i, "stepMetaData", {
            userId,
            isRebaseable,
            createdAt: Date.now()
        }))
    });
    if (!tracers) {
        tr.setMeta(tracers, traceData))
    } else {
        tracers.forEach(trace => {
            // we copy the trace data for collab rebases
            // and redoes/undoes UNLESS the tracer metadata
            // specifically designates us not to
            if (!tr.steps[trace.index] instanceof NoOpStep || !trace.value.isRebaseable || ['undo'].includes(trace.event)) {
               traceData[trace.index].value = trace.value;
            }
        })
    }
    myCustomBackend
        .send(traceData)
        .then(() => console.warn('per-transaction metadata sent'))
}

In this particular implementation, the idea is that depending on some combination of the trace data itself, and the union type of the event (which in my thinking could come from prosemirror-collab vs prosemirror-history....not sure the best way to represent this), determines whether or not we copy the trace data that came from the original transaction, vs. using the "fresh" trace data.

I'm not totally sure the best way to represent the event to allow for both history/collab...maybe this could also be namespaced to the plugin that ends up copying/preserving the trace data?

[marijnh]

Hm. Would it make sense for the collab module, when it rebases steps, to add tracers for those steps twice—once when it reverts them at the start of the rebase, and once when it re-applies them at the end?

I think, since the number of different actions that involve re-applying past steps is quite limited, we should be fine with just using strings for the event types.

[marijnh]

How does this revision look to you?

[saranrapjs]

I think this looks good; so for the average rebase, where a step isn't dropped, you'd end up with two traces — one for rebase-invert and rebase-reapply?

[marijnh]

Yes. That's slightly awkward, in that you have to check whether both or only one tracer is present for a step to determine what happened to it, but it seems the most straightforward way to describe what's going on.

[bradleyayers]

My use-case for this would be tracking ranges over a document. Presumably I'd store the range information in plugin state, and then update when the plugin is given the transaction. I think there should be enough information available through tracers to do this. Having a NoOp step doesn't phase me, it seems like a reasonable compromise.

[marijnh]

My use-case for this would be tracking ranges over a document.

What kind of ranges, and how do individual steps come into that? Is this something like adding a comment/annotation, and being able to undo and redo that?

[marijnh]

Following discussion with @saranrapjs , I've updated the proposal to increase its scope a bit—the collab module could also use tracers to provide more information about rebased steps.

[vladminsky]

My use-case for this would be tracking ranges over a document.

What kind of ranges, and how do individual steps come into that? Is this something like adding a comment/annotation, and being able to undo and redo that?

@marijnh just FYI my particular use case looks similar to what you described.

Comments plugin is implemented using decorations. Once a user deletes a text with a comment decorations and then undo deletion - text is restored while comments are lost.

[marijnh]

I see. That case wouldn't be straightforwardly covered by this feature, unfortunately—the steps that delete the commented text are likely created by code that doesn't know about comments, and thus won't be adding the tracers to indicate that a given step deletes a comment (which you could then use to restore it when the step is undone).

That is a common use case though, and we should probably try to incorporate it in the proposal. It seems like it would be necessary for plugins to somehow add tracers to any transaction that comes in, maybe through a hook that's called right after the transaction filter.

That seems a bit random, and I don't have time today to think about it more deeply, but I guess it should be safe (adding trace metadata shouldn't conflict with any other functionality).

[marijnh]

Patch 6a69d90 adds a plugin hook attachTracers that can be used to add tracers to transactions coming from other sources.

[saranrapjs]

To be clear: when undoing steps that have tracers associated with them, the existing spec already will re-add tracers, but this new capability is to support cases where the metadata may be specific to the content of the steps? Is attachTracers implemented somewhere we can see? (mostly just trying to get a sense for whether we need this vs. if just attaching tracers when dispatching the transaction will suffice)

[marijnh]

No, this hasn't been implemented yet. But basically, the transaction apply driver will, before applying each transaction, see if any plugins want to add tracers, and if so, add them to the transaction's existing set of tracers. So any downstream code (such as state field apply methods) will see the full set of tracers.

That does mean that attachTracer callbacks may in some cases have to check whether a tracer already exists—for example if you delete a range that had a comment associated with it, a plugin might add information about that comment as a tracer. If that step is then undone and redone, the plugin will see the same step again deleting a comment, but since it still has that initial tracer, you probably don't want to add another instance of it. That's awkward, but, I guess, workable.

[marijnh]

There's now an implementation of the current state of the RFC in the tracer branches of the state, history, and collab repositories.

If you're interested in this, it'd be wonderful if you could try it out and see if it actually works for you.

[marijnh]

Has anybody found the time yet to try to build a proof-of-concept of their desired feature on top of this? I'd love to have some feedback from someone who's tried to actually use it before I finalize this.

[saranrapjs]

Hey @marijnh we've been swamped but I think we can find time to do this early next week

[bradleyayers]

Unfortunately I haven't either.

[VishalBhanderi]

Any update on this functionality? Would be super useful for tracking comments properly

[marijnh]

Given that I never received concrete feedback, and that the proposal feels a bit heavyweight and awkward, I didn't proceed with this. I've recently been working on the same problem in CodeMirror version 6, where it ended up working quite differently.

My current thinking is that the ideas in this RFC aren't the way to go. An alternative approach using custom step types might work better, but in order to track comment deletion through content deletion you'd still need to kludge something appendTransaction in order to add the step that indicates a comment was deleted. But because you can only append after transactions, the step order will be awkward (the comment's context no longer exists where you add the step, so it's tricky to reliably store its position).

I guess the summary is that this needs a lot more work, which I'm unlikely to put in on the short term.

[Nantris]

This would be really nice if it would enable us to preserve the proper decorations upon undo. Currently I can't see a way to restore the decorations when undoing.

@marijnh is there any chance this will be revisited? We're using TipTap so it's probably not plausible for us to try the old branches you mentioned above but I'd be happy to test it out otherwise.