Refactor Commands into Transform Helpers


#1

prosemirror-commands provides many very helpful commands for joining/lifting/etc. In the process of building complex commands, I find myself wanting to use these builtin commands as building blocks. Unfortunately their API is to take state and onAction and actually apply the changes. I propose instead to refactor these commands into methods that operate on a transform and returns either false (could not execute command) or the new transform. This would allow for easier chaining of builtin commands in creating custom commands.

As an example of one of these custom commands, I have a command that will lift all blocks within selection and then wrap selection in provided block type. This works by operating on a transform to lift all blocks and then calling wrapIn on the transform.selection. I’d like to call the wrapIn provided but that takes a state and I need to operate on my transform which has everything lifted. With this command I can turn a selection into a blockquote and remove bullets & headings.

Thoughts?


Automatic joining of lists, blockquotes, and similar
#2

Note that while this definitely holds for some of the stuff that the commands package does, joining and lifting, specifically, are based on primitives from the transform package which are exported.

This, for example, really shouldn’t be hard to write on top of liftTarget, Transform.lift, and Transform.setBlockType.

Returning a transform from a command isn’t going to work, because commands, as they work now, can create any action, not just transformations. What you could do, in some cases, is pass a custom onAction callback to a recursively called command, but that’s not super straightforward.

The idea is that commands are interface units that you bind to keys or menu items, which do one thing and perform all the checking of whether that thing makes sense, and for that the current interface works well. Utilities to be used by commands are a separate thing, and I’m open to factoring some of those into a more reusable form.


#3

Absolutely. I’m not suggesting the commands interface changes. More like, the commands that exist now would be wrappers around transform utility functions.


#4

Does the introduction of Transaction change the landscape enough for this to be reconsidered? I can appreciate that commands are more powerful than transforms, because transforms don’t encapsulate all the state that’s in play (e.g. stored marks), but from what I’ve gathered so far transactions do?

Ultimately I think my use-case is the same as @wes-r — I want composable mutations. I think I want this so that I have full control over where history events (by treating each transaction as a history event, and being responsible for building up a single transaction of appropriate changes before dispatching it).

What I have in mind is an interface like this:

interface Mutator {
    // Apply the mutation to a transaction, returning the resulting transaction.
    // If the mutation can't be applied, returning the original transaction unchanged.
    apply(tr: Transaction): Transaction;
    // Return true if the mutation can be applied.
    canApply(tr: Transaction): boolean;
}

It’s very similar to a command as they exist today, but it doesn’t know anything about the view and dispatching.


#5

The command interface isn’t going to be overhauled again, for the sake finally stabilizing but also because I think their current form expresses what they do better (they may have side effects, beyond dispatching a transaction).

You can extend commands with this slightly awkward but unproblematic pattern:

function andSelectAll(command) {
  return (state, dispatch) => {
    return command(state, dispatch && (tr => {
      if (tr.isGeneric) // Don't touch non-generic transactions
        tr.setSelection(Selection.between(tr.doc.resolve(0),
                                          tr.doc.resolve(tr.doc.content.size)))
      dispatch(tr)
    }))
  }
}

[Edit: Actually balance brackets]


#6

I’m happy for commands to stay as they are, but what I’d propose is decomposing the implementation into “mutators” (open to a better name) that are composable. The end result should be commands that maintain their current interface, but whose internals are probably just a couple of lines combining mutators and calling dispatch.

I think I’m missing something regarding the difference between transactions and editor state, specifically regarding what state they encapsulate. What’s missing from transactions that prevent commands from only working with transactions? It’s surprising to me that it’s not an explicit goal for commands to work on transactions, so I believe there’s some consideration that I’m overlooking.

I suspect we made a mistake with this thread in conflating transforms with commands — commands can continue to live as-is, but (I think what @wes-r and) I want is something new that’s similar to commands, but is a lower-level composable construct. In practice I’d probably end up never using commands, and always build my own because of specific user-experience requirements that I’m aiming for.


#7

Having two functions, one to check whether something applies and one to execute it, is how things initially worked but it was really hard to keep those two consistent, so I moved to the one-function-with-optional-dispatch form, and it made the code much simpler and less error prone.

Adding another concept of helpers that implement the functionality of the commands is possible, but I’m not really convinced the added value is worth the extra API surface.


#8

FWIW I came across this thread as I was running into the exact same problem - I’ve been using the built in commands, and they’re great up until I need something slightly different, or I need to compose a few of them together, but ultimately I’m having to just copy the internals of them out of prosemirror-commands and reimplement locally.

@bradleyayers idea of a lower level composable interface would be ideal for me, and I think I’m in a similar boat - I doubt I’d end up actually using any of the built in commands, as nearly all the behaviours I’m after are just slightly different to the built in commands.

For me, I think it’s worthwhile to have the extra API surface - IMO I don’t think it would be too much cognitive overhead for people coming to ProseMirror, and it would be incredibly worthwhile. I found it really confusing that I couldn’t chain together commands (until I properly understood their inner workings) - indeed that’s what I thought the chainCommands helper initially did. Having a different set of ‘utilities’ would help clear this up I think.


#9

Ping @marijn - I hit this again! The liftListItem implementation in prosemirror-schema-list is awesome as it will do the right thing for any nested list items. The issue I ran into is wanting to invoke it several times to lift an item ALL THE WAY out of the list (a “remove” command). Presently there is no good way to do this. I had to copy the implementation and modify it to operate on a transaction so that I can invoke it in a loop.

All of this great functionality is locked away behind dispatched commands. Instead they should be building blocks! Seems many of us long term users have solved this by copying the innards of the commands we want to build upon. This is not good! It makes managing our code harder and it adds a barrier to entry for beginners as they will inevitably run into the same desire and the same less-than-perfect-solution.

35%20AM —> 48%20AM


#10

I definitely agree that there are a few utility-type functions that I’ve either copied and pasted from the core lib or written from scratch and subsequently realised that I’d reimplemented something in that was already in the core library.


#11

Also – there’s no reason to drastically increase the API surface area. If all of the existing commands operated on transactions and returned one, then there could be a new high level command helper dispatcher() or something like that, so dispatcher(joinBackward()) would be equivalent to the joinBackward that exists today.


#12

You usually don’t want the exact command as a helper, though. Where a command will act on the selection, a helper tends to work better when it takes arguments that tell it where to apply its effect.

But I’m okay with growing the API surface a little to expose reusable code where it’s appropriate. Do you want to propose a pull request that splits the guts of liftListItem into an exported utility function?


#13

YES! I’ll hopefully have something for you next week.

Can you elaborate on this? or provide an example of the signature you’d like to see for this helper? I’m currently using:

export function liftListItem(itemType) {
  return function(selection, tr) {
    ...

#14

I have been exploring the idea of using transforms for a while and it seems to be working quite well for us. Here are a few examples: https://github.com/atlassian/prosemirror-utils/blob/master/src/transforms.js

I was going to decompose PM commands into utils in a similar way as you described, I just didn’t have enough time so far to do that :slight_smile:


#15

Passing a selection makes sense. Or it could just be a pair of positions. Also, I’d include itemType directly in the parameters of the function—if we don’t have to correspond to the command function signature, there’s not need for the wrapping factory function.


How to use default ListItem attributes when splitting a ListItem