Refactor Commands into Transform Helpers

21

Trying to wrap my head around the precise use case of calling a command from another command: can’t this be achieved by passing down a stateful dispatch function?

Something along those lines:

function myCmd() {
   return cmd(state, dispatch) {
      if (!dispatch) return false
      const transactions = []
      function nestedDispatch(tr) {
         transactiosn.push(tr)
      }
     // using an existing command (in more complex code this might depend of state etc.)
      toggleMark(someMark)(state, nestedDispatch)
      const mergedTransaction = transactions
        .reduce(/* merge transactions into one somehow ?*/)
      dispatch(mergedTransaction)
   }  
}

So my question would be: can we marge transactions afterwards?

Without a pattern to compose commands, I’ve noticed our code tends to create a lot of transactions by running multiple commands where conceptually only one is expected, which may then have a weird effect on undo/redo. For instance, running some commands in a loop.

22

In ProseMirror’s approach, commands are actual self-contained user actions. The idea is that you write helper functions to implement the logic that you need to share between commands, and commands themselves aren’t composable. Because transactions fundamentally can’t be blindly merged—they may set metadata that breaks if additional changes or effects are included in the same transaction.

I know Tiptap is less careful about this distinction, but I suspect that will cause issues in some cases.

23

Thanks for your fast answer as always @marijn !

So for instance let say I want to implement a command that remove “bold”, “italic” and “underline” markes in one go (but keep other marks around). For this use case you would rather reuse some logic similar to toggleMark implementation to craft a new command, rather than trying to find a hack to trigger 3 toggleMark calls in the same transaction? (trying to rephrase on a basic example for extra clarity because the discussions above are more conceptual)

24

Correct. Unfortunately, some of the commands in prosemirror-commands do rather non-trivial things and don’t expose related helper functions (yet). But if you see a piece of functionality that looks like it should be factored out into an exported utility function, feel free to propose that.

1 Like
25

Here is an example of what it can look like:

  • wrapInListTr is just the same logic as wrapInList as it currently exist in the codebase, but it takes the transation as a parameter
  • wrapInList takes care of initiating the transaction, but that’s all
export function wrapInListTr(tr: Transaction, listType: NodeType, attrs: Attrs | null = null): Transaction | false {
  let { $from, $to } = tr.selection
  let range = $from.blockRange($to), doJoin = false, outerRange = range
  if (!range) return false
  // This is at the top of an existing list item
  if (range.depth >= 2 && $from.node(range.depth - 1).type.compatibleContent(listType) && range.startIndex == 0) {
    // Don't do anything if this is the top of the list
    if ($from.index(range.depth - 1) == 0) return false
    let $insert = tr.doc.resolve(range.start - 2)
    outerRange = new NodeRange($insert, $insert, range.depth)
    if (range.endIndex < range.parent.childCount)
      range = new NodeRange($from, tr.doc.resolve($to.end(range.depth)), range.depth)
    doJoin = true
  }
  let wrap = findWrapping(outerRange!, listType, attrs, range)
  if (!wrap) return false
  //if (dispatch) dispatch(doWrapInList(state.tr, range, wrap, doJoin, listType).scrollIntoView())
  return doWrapInList(tr, range, wrap, doJoin, listType).scrollIntoView()
}

/// Returns a command function that wraps the selection in a list with
/// the given type an attributes. If `dispatch` is null, only return a
/// value to indicate whether this is possible, but don't actually
/// perform the change.
export function wrapInList(listType: NodeType, attrs: Attrs | null = null): Command {
  return function (state: EditorState, dispatch?: (tr: Transaction) => void) {
    const tr = wrapInListTr(state.tr, listType, attrs)
    if (!tr) return false
    if (dispatch) dispatch(tr)
    return true
  }
}

and here is how it can be used to swap list type (removing previously set list type, and applying wanted one) in a single command:

function swapListType (state, dispatch) {
        // remove other type of list if any
        let tr: Transaction | false = state.tr
        for (const markType of listNodes) {
            if (innerNode.type === markType) {
                // note that I have refactored "liftListItem" too
                // to accept a transaction
                tr = liftListItemTr(tr, li)
                if (!tr) return false
                break
            }
        }
        // apply wanted type
        tr = wrapInListTr(tr, mt)
        if (tr) {
            dispatch(tr)
            return true
        }
        return false
    }

@marijn any opinion on this?

1 Like