Code review feedback

- Use config instead of spec to describe
the parameters passed to the markdown
serializer
- Rename document variable to proseMirrorDocument
- Wrap document, dropCursor, gapCursor, and history
extensions
- Extract build_serializer_config into a different
service and add specs

app/assets/javascripts/content_editor/extensions/document.js

+import Document from '@tiptap/extension-document';
+
+export const tiptapExtension = Document;

app/assets/javascripts/content_editor/extensions/dropcursor.js

+import Dropcursor from '@tiptap/extension-dropcursor';
+
+export const tiptapExtension = Dropcursor;

app/assets/javascripts/content_editor/extensions/gapcursor.js

+import Gapcursor from '@tiptap/extension-gapcursor';
+
+export const tiptapExtension = Gapcursor;

app/assets/javascripts/content_editor/extensions/history.js

+import History from '@tiptap/extension-history';
+
+export const tiptapExtension = History;

app/assets/javascripts/content_editor/services/build_serializer_config.js

+const buildSerializerConfig = (extensions = []) =>
+  extensions
+    .filter(({ serializer }) => serializer)
+    .reduce(
+      (serializers, { serializer, tiptapExtension: { name, type } }) => {
+        const collection = `${type}s`;
+
+        return {
+          ...serializers,
+          [collection]: {
+            ...serializers[collection],
+            [name]: serializer,
+          },
+        };
+      },
+      {
+        nodes: {},
+        marks: {},
+      },
+    );
+
+export default buildSerializerConfig;

app/assets/javascripts/content_editor/services/create_content_editor.js

-import Document from '@tiptap/extension-document';
-import Dropcursor from '@tiptap/extension-dropcursor';
-import Gapcursor from '@tiptap/extension-gapcursor';
 import { Editor } from '@tiptap/vue-2';
 import { isFunction } from 'lodash';
 import { PROVIDE_SERIALIZER_OR_RENDERER_ERROR } from '../constants';
@@ -9,8 +6,12 @@ import * as Bold from '../extensions/bold';
 import * as BulletList from '../extensions/bullet_list';
 import * as Code from '../extensions/code';
 import * as CodeBlockHighlight from '../extensions/code_block_highlight';
+import * as Document from '../extensions/document';
+import * as Dropcursor from '../extensions/dropcursor';
+import * as Gapcursor from '../extensions/gapcursor';
 import * as HardBreak from '../extensions/hard_break';
 import * as Heading from '../extensions/heading';
+import * as History from '../extensions/history';
 import * as HorizontalRule from '../extensions/horizontal_rule';
 import * as Image from '../extensions/image';
 import * as Italic from '../extensions/italic';
@@ -19,6 +20,7 @@ import * as ListItem from '../extensions/list_item';
 import * as OrderedList from '../extensions/ordered_list';
 import * as Paragraph from '../extensions/paragraph';
 import * as Text from '../extensions/text';
+import buildSerializerConfig from './build_serializer_config';
 import { ContentEditor } from './content_editor';
 import createMarkdownSerializer from './markdown_serializer';
 
@@ -28,12 +30,15 @@ const builtInContentEditorExtensions = [
   BulletList,
   Code,
   CodeBlockHighlight,
-  BulletList,
+  Document,
+  Dropcursor,
+  Gapcursor,
   HardBreak,
   Heading,
+  History,
   HorizontalRule,
-  Italic,
   Image,
+  Italic,
   Link,
   ListItem,
   OrderedList,
@@ -44,30 +49,9 @@ const builtInContentEditorExtensions = [
 const collectTiptapExtensions = (extensions = []) =>
   extensions.map(({ tiptapExtension }) => tiptapExtension);
 
-const buildSerializerSpec = (extensions = []) =>
-  extensions
-    .filter(({ serializer }) => serializer)
-    .reduce(
-      (serializers, { serializer, tiptapExtension: { name, type } }) => {
-        const collection = `${type}s`;
-
-        return {
-          ...serializers,
-          [collection]: {
-            ...serializers[collection],
-            [name]: serializer,
-          },
-        };
-      },
-      {
-        nodes: {},
-        marks: {},
-      },
-    );
-
 const createTiptapEditor = ({ extensions = [], ...options } = {}) =>
   new Editor({
-    extensions: [Dropcursor, Gapcursor, History, Document, ...extensions],
+    extensions: [...extensions],
     editorProps: {
       attributes: {
         class: 'gl-outline-0!',
@@ -84,8 +68,8 @@ export const createContentEditor = ({ renderMarkdown, extensions = [], tiptapOpt
   const allExtensions = [...builtInContentEditorExtensions, ...extensions];
   const tiptapExtensions = collectTiptapExtensions(allExtensions);
   const tiptapEditor = createTiptapEditor({ extensions: tiptapExtensions, ...tiptapOptions });
-  const serializerSpec = buildSerializerSpec(allExtensions);
-  const serializer = createMarkdownSerializer({ render: renderMarkdown, serializerSpec });
+  const serializerConfig = buildSerializerConfig(allExtensions);
+  const serializer = createMarkdownSerializer({ render: renderMarkdown, serializerConfig });
 
   return new ContentEditor({ tiptapEditor, serializer });
 };

app/assets/javascripts/content_editor/services/markdown_serializer.js

@@ -15,7 +15,7 @@ const wrapHtmlPayload = (payload) => `<div>${payload}</div>`;
  * that parses the Markdown and converts it into HTML.
  * @returns a markdown serializer
  */
-export default ({ render = () => null, serializerSpec }) => ({
+export default ({ render = () => null, serializerConfig }) => ({
   /**
    * Converts a Markdown string into a ProseMirror JSONDocument based
    * on a ProseMirror schema.
@@ -49,11 +49,11 @@ export default ({ render = () => null, serializerSpec }) => ({
    * @returns A Markdown string
    */
   serialize: ({ schema, content }) => {
-    const document = schema.nodeFromJSON(content);
-    const { nodes, marks } = serializerSpec;
+    const proseMirrorDocument = schema.nodeFromJSON(content);
+    const { nodes, marks } = serializerConfig;
     const serializer = new ProseMirrorMarkdownSerializer(nodes, marks);
 
-    return serializer.serialize(document, {
+    return serializer.serialize(proseMirrorDocument, {
       tightLists: true,
     });
   },

spec/frontend/content_editor/services/build_serializer_config_spec.js

+import * as Blockquote from '~/content_editor/extensions/blockquote';
+import * as Bold from '~/content_editor/extensions/bold';
+import * as Dropcursor from '~/content_editor/extensions/dropcursor';
+import * as Paragraph from '~/content_editor/extensions/paragraph';
+
+import buildSerializerConfig from '~/content_editor/services/build_serializer_config';
+
+describe('content_editor/services/build_serializer_config', () => {
+  describe('given one or more content editor extensions', () => {
+    it('creates a serializer config that collects all extension serializers by type', () => {
+      const extensions = [Bold, Blockquote, Paragraph];
+      const serializerConfig = buildSerializerConfig(extensions);
+
+      extensions.forEach(({ tiptapExtension, serializer }) => {
+        const { name, type } = tiptapExtension;
+        expect(serializerConfig[`${type}s`][name]).toBe(serializer);
+      });
+    });
+  });
+
+  describe('given an extension without serializer', () => {
+    it('does not include the extension in the serializer config', () => {
+      const serializerConfig = buildSerializerConfig([Dropcursor]);
+
+      expect(serializerConfig.marks[Dropcursor.tiptapExtension.name]).toBe(undefined);
+      expect(serializerConfig.nodes[Dropcursor.tiptapExtension.name]).toBe(undefined);
+    });
+  });
+
+  describe('given no extensions', () => {
+    it('creates an empty serializer config', () => {
+      expect(buildSerializerConfig()).toStrictEqual({
+        marks: {},
+        nodes: {},
+      });
+    });
+  });
+});