Using prosemirror-inputrules[undoInputRule] with prosemirror-history

#1

Hey!

I’ve been working around the UX for undoing input-rules in my editor. I’m trying to get the following behaviour to occur

  1. Type “*this is strong”
  2. Type “*”
  3. Results in “this is strong”
  4. Hit undo: “*this is strong*”
  5. Hit undo a second time: “*this is strong”

To get the undo behaviour, my current approach uses both prosemirror-inputrules & prosemirror-history, which chains undoInputRule & the standard undo command to look like

const keymaps = keymap({
  'Mod-z': chainCommands(undoInputRule, undo)
})

However this ends up looking like:

  1. Type “*this is strong”
  2. Type “*”
  3. Results in “this is strong”
  4. Hit undo: “*this is strong*”
  5. Hit undo a second time: “this is strong”
  6. Hit undo a third time: “*this is strong”

autoformatting

I worked out this is happening because while undoInputRule reverses the input rule - it achieves this by keeping track of the last input rule transformation (unaware of prosemirror-history).

To get it the way describe in the first example, it seems like I’d have to somehow get the undoInputRule transaction to be added to the redo stack. But I feel like that’d complicate collab editing?

I’ve got an example you can play around with here

#2

There’s not currently a way to add a non-undo transaction to the redo stack. But if you can do without that, wrapping undoInputRule to add an addToHistory=false meta property, it would at least not be added to the undo history.

Alternatively, you could try forking the inputrules plugin to make it work differently by first allowing the text input to go through, and then, in a separate transaction, apply the rule’s effect. That way undo would work automatically.

#3

Thanks for the fast response :slight_smile: that’s what I was thinking of, I had a go at both approaches yesterday:

undoInputHistory: addToHistory=false

I tried not adding it to the undo stack, but there’s a strange selection error that gets thrown inside history if you undo twice, then attempt to redo: https://wandering-range.glitch.me/ (https://glitch.com/edit/#!/wandering-range?path=index.js) - could be an issue trying to reapply the undone transaction?

Forking input-rules

I had a go with forking the input-rules - it’s gets a bit tricky as we have the following case where if we wanted to modify the existing code to apply the text first, we need a way of being sure that can apply a transaction to this state rules[i].handler. :

          let match = rules[i].match.exec(textBefore)
          tr = match && rules[i].handler(state, match, from - (match[0].length - text.length), to)
          if (!tr) continue
          view.dispatch(tr.setMeta(this, ...));

My first naive attempt obviously failed because the original transaction the handler returned was bound to the state before we inserted the text :man_facepalming:

          let match = rules[i].match.exec(textBefore)
          tr = match && rules[i].handler(state, match, from - (match[0].length - text.length), to)
          if (!tr) continue
          view.dispatch(view.state.tr.insertText(text));
          view.dispatch(tr.setMeta(this, ...));

So I came up with something a little different (I’m so sorry :stuck_out_tongue: ). I made the InputRule handler accept a transaction (instead of state), and pass it a transaction with the text state already applied (since we’re now applying the text first, the handler needs to know what the doc will look like).

          let match = rules[i].match.exec(textBefore)
          // Creates a tr with a view of what the doc will/could be
          let tr = state.tr.insertText(text) as any; 
          tr = match && rules[i].handler(tr, match, from - (match[0].length - text.length), to + text.length)
          if (!tr) continue
          // Applies a new tr (as the old one changed)
          view.dispatch(state.tr.insertText(text));
          view.dispatch(rules[i].handler(view.state.tr, match!, from - (match![0].length - text.length), to + text.length));

This works - with minor changes to existing input rules - though my spidey-senses may be incorrectly firing when I feel like this isn’t the safest (or nicest way) to achieve this. I’ll probably continue having a look into this to see whether there’s a better way to structure this.

Other thoughts

From playing around with this though (and I’ll prempt this with “this is not a feature request” :wink: ) I did wonder whether it’d be possible to use a “HistoryStep” to flag to the history plugin that it should create a new undo marker in the middle of a transaction, i.e.

view.dispatch(
  state.tr
    .insertText('replacement')
    .step(HistoryStep.create())
    .replaceRangeWith(...)
)

It does come with the caveat that it doesn’t conceptually fulfil the purpose of a step to transform the document, but I feel it’d be a powerful way to support transactions that want to allow the users to rollback part of it on an undo.

#4

I’ll look into that. But even without the error, this was a terrible suggestion of me—with this flag set, the changes will stick around when you undo, which is really not what we wanted here.

Wouldn’t this also be solved by dispatching multiple separate transactions?

#5

prosemirror-history 1.0.1 should fix the error that you saw (though, again, you probably don’t want to use that approach anyway)

#6

prosemirror-history 1.0.1 should fix the error that you saw (though, again, you probably don’t want to use that approach anyway)

Yay for incidental edge-case discovery :wink:

Wouldn’t this also be solved by dispatching multiple separate transactions?

I tried to explain that above, but maybe I didn’t do a good job of it. I could be making a bad assumption, but if I had code like:

view.dispatch(view.state.tr.insertText('bananas'));
view.dispatch(view.state.tr.replaceBananasWithApples())

Am I being too overly cautious in thinking that, it would be possible for appendTransaction to be invoked in between these two transactions (potentially resulting in the action() not being able to be applied successfully to the state? I think I’m just seeing the dispatches and thinking of async related bugs popping up, i.e. another transaction sneaking in / document changing in the interim.

In the case of input-rules usecase, we’d need to call the input-rule handler twice (unless I’m missing something):

  1. to see whether we should add the user’s text to the document at all. (We have to do this because there might be another input-rule that should applied instead of the current one if it can’t generate a transaction)
  2. to actually generate the transaction to apply the transformation
#7

That’s true, that might be an issue if you dispatch them like this. But in a plugin’s handleTextInput you have a reference to the view object, so you can fetch the current state, after the first dispatch, from that.

But yeah, to be safe you’d have to run the text match twice, to make sure it still matches after the transaction.

#8

There can’t be any async code that can modify the state between the calls to dispatch (appendTransaction is sync), however in a collab environment it could be possible for a collaborator to confirm steps between the two transactions (though this would only be discovered after the second transaction has been dispatched and would be resolved via rebasing).

Regardless the only concern you really need to have is the effect of appendTransaction, but I haven’t seen any code in practice that would cause a problem here.

I think something like this would be very useful.

#9

As far as I have read the code, when one does this:

let newState = view.state.apply(tr)

and the transaction by means of a plugin creates a second or third transaction, newState will be set to the state of the last two or three transactions applied. That then means that there really isn’t any way that a collaborator could insert a transaction between the two. When one sends it to collaborators afterward, it will just be sent as a number of steps which will be approved by the central authority once and on the other end of other collaborators will be turned into a single, multi-step transaction.

Or is there something there I didn’t get?

#10

Yes view.state.apply(tr) is all synchronous, and unless your collab code sends steps separately to the central authority, there isn’t a way for a collaborator it interleave steps into the ones created locally.

However if you call view.dispatch(…) twice, unless you’re debouncing sending steps to a central authority, there’s no guarantee that a collaborator won’t inject steps between the two separately dispatched transactions.

#11

All of JavaScript within a normal browser is single-threaded, right? So if you call dispatch twice in a row, how could there then simultaneously be new incoming steps? That the central authority (another computer) receives packages from others at that same time should not matter. The station that the user is at should apply all the steps in order before it listens to anything incoming, right? So for all intents and purposes when dealing with collaborators, it’s the same as if it were a single, multi-step transaction, as far as I can tell.

Or is dispatch using setTimeout() or some such thing?

#12

That’s right yes. Locally you would never immediately see any foreign steps between dispatching two transactions, because as you say JavaScript is single threaded. However sending steps to the authority is asynchronous, and there’s some time between sending the steps from the first transaction, and those from the second. So what might happen is sending the second set of steps might be rejected by the authority, and you would need to rebase them.

#13

Gotcha. ok, but then again, you could just set it a variable like this.dontSend=true up that your sender checks for so that you don’t send anything accidentally until you’re entirely done.

#14

Yes definitely, or by debouncing sending steps to the authority (which I do to get some basic batching of keystrokes). I use 250ms with an exponential back-off.

#15

Similar here. The exponential bit turned out to be the safest ways of handling cats sleeping on keyboards, etc.