ueberdosis / tiptap

The headless rich text editor framework for web artisans.
https://tiptap.dev
MIT License
27.36k stars 2.27k forks source link

Tiptap editor become frozen when extending Paragraph extension #1288

Closed enriquecastl closed 3 years ago

enriquecastl commented 3 years ago

Description

The editor become frozen and completely unusable when the Paragraph extension is extended:

const customParagraph = Paragraph.extend();

However, if I pass the parent Paragraph extension’s config to the child extension, this bug is fixed:

const customParagraph = Paragraph.extend({
  ...Paragraph.config,
});

CodeSandbox

I created a CodeSandbox to help you debug the issue. It demonstrates how I set up the tiptap v2 editor and you can reproduce the issue: https://codesandbox.io/s/bug-infinite-loop-tiptap-v2-extend-paragraph-wkj58

Expected behavior The paragraph extension can be extended without breaking the editor

Environment?

Additional context Add any other context about the problem here.

hanspagel commented 3 years ago

Why would you do that? 🤔 I’m not really sure what’s the use case here.

And would this.parent help for that use case?

const CustomTableCell = TableCell.extend({
  addAttributes() {
    return {
      ...this.parent?.(),
      myCustomAttribute: {
        // …
      },
    }
  },
})

https://www.tiptap.dev/guide/custom-extensions

enriquecastl commented 3 years ago

Hi @hanspagel 👋🏼

Why would you do that?

Do you mean why I would extend the Paragraph extension?

I’m extending the paragraph extension to instrument the execution of keyboard shortcuts in the Editor. The small code snippet I shared in the bug description doesn’t represent my use case. It only represents the code that triggers the bug.

This is the WIP MR where I am implementing instrumentation: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61248/

This is the instrumentation module. It extends the extension’s addKeyboardShortcuts to decorate the function that executes the command with a function that emits a tracking event:

import { mapValues, isFunction } from 'lodash';
import Tracking from '~/tracking';
import { CONTENT_EDITOR_TRACKING_LABEL, KEYBOARD_SHORTCUT_TRACKING_ACTION } from '../constants';

const trackKeyboardShortcut = (contentType, commandFn, shortcut) => () => {
  Tracking.event(undefined, KEYBOARD_SHORTCUT_TRACKING_ACTION, {
    label: CONTENT_EDITOR_TRACKING_LABEL,
    property: `${contentType}.${shortcut}`,
  });
  return commandFn();
};

const trackInputRulesAndShortcuts = (tiptapExtension) => {
  if (!isFunction(tiptapExtension.config.addKeyboardShortcuts)) {
    return tiptapExtension;
  }

  const result = tiptapExtension.extend({
    /**
     * Note: we shouldn’t pass the parent extension config
     * to the children because it should be automatically
     * inherited.
     *
     * However we found an issue where the editor becomes
     * frozen if the paragraph extension is extended
     * without passing the parent config explicitely
     *
     * We reported the issue in the tiptap repository
     * https://github.com/ueberdosis/tiptap/issues/1288
     */
    ...tiptapExtension.config,
    addKeyboardShortcuts() {
      const shortcuts = this.parent();
      const { name } = this;
      const decorated = mapValues(shortcuts, (commandFn, shortcut) =>
        trackKeyboardShortcut(name, commandFn, shortcut),
      );

      return decorated;
    },
  });

  return result;
};

export default trackInputRulesAndShortcuts;
philippkuehn commented 3 years ago

There was an issue that the priority was getting lost on extending which led to this bug. Should now work without ...Paragraph.config.

BTW this is not a safe check:

if (!isFunction(tiptapExtension.config.addKeyboardShortcuts)) {
    return tiptapExtension;
}

This is because an extension.config holds only the config for the current extension. If the extension has been extended, it may be that the needed field has been defined only in extension.parent.config or in extension.parent.parent.config or extension.parent.parent.parent.config

enriquecastl commented 3 years ago

Thanks so much for the quick fix, @philippkuehn 🙇🏼 . Also, thanks for the pro-tip regarding the existence of keyboard shortcuts. I will find a better approach for sure.