Merge branch '217801-step-1-update-editor-lite' into 'master'

Step 1 - Update editor lite and component with global id

See merge request gitlab-org/gitlab!39232

app/assets/javascripts/blob/components/blob_edit_content.vue

@@ -20,6 +20,13 @@ export default {
       required: false,
       default: '',
     },
+    // This is used to help uniquely create a monaco model
+    // even if two blob's share a file path.
+    fileGlobalId: {
+      type: String,
+      required: false,
+      default: '',
+    },
   },
   data() {
     return {
@@ -36,7 +43,11 @@ export default {
       el: this.$refs.editor,
       blobPath: this.fileName,
       blobContent: this.value,
+      blobGlobalId: this.fileGlobalId,
     });
+
+    this.editor.onChangeContent(debounce(this.onFileChange.bind(this), 250));
+
     window.requestAnimationFrame(() => {
       if (!performance.getEntriesByName(SNIPPET_MARK_BLOBS_CONTENT).length) {
         performance.mark(SNIPPET_MARK_BLOBS_CONTENT);
@@ -45,16 +56,19 @@ export default {
       }
     });
   },
+  beforeDestroy() {
+    this.editor.dispose();
+  },
   methods: {
-    triggerFileChange: debounce(function debouncedFileChange() {
+    onFileChange() {
       this.$emit('input', this.editor.getValue());
-    }, 250),
+    },
   },
 };
 </script>
 <template>
   <div class="file-content code">
-    <div id="editor" ref="editor" data-editor-loading @keyup="triggerFileChange">
+    <div id="editor" ref="editor" data-editor-loading>
       <pre class="editor-loading-content">{{ value }}</pre>
     </div>
   </div>

app/assets/javascripts/blob/utils.js

 import Editor from '~/editor/editor_lite';
 
-export function initEditorLite({ el, blobPath, blobContent }) {
+export function initEditorLite({ el, ...args }) {
   if (!el) {
     throw new Error(`"el" parameter is required to initialize Editor`);
   }
   const editor = new Editor();
   editor.createInstance({
     el,
-    blobPath,
-    blobContent,
+    ...args,
   });
 
   return editor;

app/assets/javascripts/editor/editor_lite.js

@@ -3,6 +3,7 @@ import { DEFAULT_THEME, themes } from '~/ide/lib/themes';
 import languages from '~/ide/lib/languages';
 import { defaultEditorOptions } from '~/ide/lib/editor_options';
 import { registerLanguages } from '~/ide/utils';
+import { joinPaths } from '~/lib/utils/url_utility';
 import { clearDomElement } from './utils';
 
 export default class Editor {
@@ -30,7 +31,16 @@ export default class Editor {
     monacoEditor.setTheme(theme ? themeName : DEFAULT_THEME);
   }
 
-  createInstance({ el = undefined, blobPath = '', blobContent = '' } = {}) {
+  /**
+   * Creates a monaco instance with the given options.
+   *
+   * @param {Object} options Options used to initialize monaco.
+   * @param {Element} options.el The element which will be used to create the monacoEditor.
+   * @param {string} options.blobPath The path used as the URI of the model. Monaco uses the extension of this path to determine the language.
+   * @param {string} options.blobContent The content to initialize the monacoEditor.
+   * @param {string} options.blobGlobalId This is used to help globally identify monaco instances that are created with the same blobPath.
+   */
+  createInstance({ el = undefined, blobPath = '', blobContent = '', blobGlobalId = '' } = {}) {
     if (!el) return;
     this.editorEl = el;
     this.blobContent = blobContent;
@@ -38,11 +48,9 @@ export default class Editor {
 
     clearDomElement(this.editorEl);
 
-    this.model = monacoEditor.createModel(
-      this.blobContent,
-      undefined,
-      new Uri('gitlab', false, this.blobPath),
-    );
+    const uriFilePath = joinPaths('gitlab', blobGlobalId, blobPath);
+
+    this.model = monacoEditor.createModel(this.blobContent, undefined, Uri.file(uriFilePath));
 
     monacoEditor.onDidCreateEditor(this.renderEditor.bind(this));
 
@@ -51,6 +59,11 @@ export default class Editor {
   }
 
   dispose() {
+    if (this.model) {
+      this.model.dispose();
+      this.model = null;
+    }
+
     return this.instance && this.instance.dispose();
   }
 
@@ -58,6 +71,10 @@ export default class Editor {
     delete this.editorEl.dataset.editorLoading;
   }
 
+  onChangeContent(fn) {
+    return this.model.onDidChangeContent(fn);
+  }
+
   updateModelLanguage(path) {
     if (path === this.blobPath) return;
     this.blobPath = path;

app/assets/javascripts/snippets/components/snippet_blob_edit.vue

@@ -101,7 +101,7 @@ export default {
         size="lg"
         class="loading-animation prepend-top-20 append-bottom-20"
       />
-      <blob-content-edit v-else v-model="content" :file-name="filePath" />
+      <blob-content-edit v-else v-model="content" :file-global-id="id" :file-name="filePath" />
     </div>
   </div>
 </template>

spec/frontend/blob/components/blob_edit_content_spec.js

 import { shallowMount } from '@vue/test-utils';
 import BlobEditContent from '~/blob/components/blob_edit_content.vue';
-import { initEditorLite } from '~/blob/utils';
+import * as utils from '~/blob/utils';
+import Editor from '~/editor/editor_lite';
 import { nextTick } from 'vue';
 
-jest.mock('~/blob/utils', () => ({
-  initEditorLite: jest.fn(),
-}));
+jest.mock('~/editor/editor_lite');
 
 describe('Blob Header Editing', () => {
   let wrapper;
   const value = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.';
   const fileName = 'lorem.txt';
+  const fileGlobalId = 'snippet_777';
 
   function createComponent(props = {}) {
     wrapper = shallowMount(BlobEditContent, {
       propsData: {
         value,
         fileName,
+        fileGlobalId,
         ...props,
       },
     });
   }
 
   beforeEach(() => {
+    jest.spyOn(utils, 'initEditorLite');
+
     createComponent();
   });
 
@@ -30,6 +33,15 @@ describe('Blob Header Editing', () => {
     wrapper.destroy();
   });
 
+  const triggerChangeContent = val => {
+    jest.spyOn(Editor.prototype, 'getValue').mockReturnValue(val);
+    const [cb] = Editor.prototype.onChangeContent.mock.calls[0];
+
+    cb();
+
+    jest.runOnlyPendingTimers();
+  };
+
   describe('rendering', () => {
     it('matches the snapshot', () => {
       expect(wrapper.element).toMatchSnapshot();
@@ -51,18 +63,15 @@ describe('Blob Header Editing', () => {
 
     it('initialises Editor Lite', () => {
       const el = wrapper.find({ ref: 'editor' }).element;
-      expect(initEditorLite).toHaveBeenCalledWith({
+      expect(utils.initEditorLite).toHaveBeenCalledWith({
         el,
         blobPath: fileName,
+        blobGlobalId: fileGlobalId,
         blobContent: value,
       });
     });
 
     it('reacts to the changes in fileName', () => {
-      wrapper.vm.editor = {
-        updateModelLanguage: jest.fn(),
-      };
-
       const newFileName = 'ipsum.txt';
 
       wrapper.setProps({
@@ -70,21 +79,20 @@ describe('Blob Header Editing', () => {
       });
 
       return nextTick().then(() => {
-        expect(wrapper.vm.editor.updateModelLanguage).toHaveBeenCalledWith(newFileName);
+        expect(Editor.prototype.updateModelLanguage).toHaveBeenCalledWith(newFileName);
       });
     });
 
+    it('registers callback with editor onChangeContent', () => {
+      expect(Editor.prototype.onChangeContent).toHaveBeenCalledWith(expect.any(Function));
+    });
+
     it('emits input event when the blob content is changed', () => {
-      const editorEl = wrapper.find({ ref: 'editor' });
-      wrapper.vm.editor = {
-        getValue: jest.fn().mockReturnValue(value),
-      };
+      expect(wrapper.emitted().input).toBeUndefined();
 
-      editorEl.trigger('keyup');
+      triggerChangeContent(value);
 
-      return nextTick().then(() => {
-        expect(wrapper.emitted().input[0]).toEqual([value]);
-      });
+      expect(wrapper.emitted().input).toEqual([[value]]);
     });
   });
 });

spec/frontend/blob/utils_spec.js

 import Editor from '~/editor/editor_lite';
 import * as utils from '~/blob/utils';
 
-const mockCreateMonacoInstance = jest.fn();
-jest.mock('~/editor/editor_lite', () => {
-  return jest.fn().mockImplementation(() => {
-    return { createInstance: mockCreateMonacoInstance };
-  });
-});
+jest.mock('~/editor/editor_lite');
 
 describe('Blob utilities', () => {
-  beforeEach(() => {
-    Editor.mockClear();
-  });
-
   describe('initEditorLite', () => {
     let editorEl;
     const blobPath = 'foo.txt';
     const blobContent = 'Foo bar';
+    const blobGlobalId = 'snippet_777';
 
     beforeEach(() => {
-      setFixtures('<div id="editor"></div>');
-      editorEl = document.getElementById('editor');
+      editorEl = document.createElement('div');
     });
 
     describe('Monaco editor', () => {
@@ -29,25 +20,21 @@ describe('Blob utilities', () => {
         expect(Editor).toHaveBeenCalled();
       });
 
-      it('creates the instance with the passed parameters', () => {
-        utils.initEditorLite({ el: editorEl });
-        expect(mockCreateMonacoInstance.mock.calls[0]).toEqual([
-          {
+      it.each([[{}], [{ blobPath, blobContent, blobGlobalId }]])(
+        'creates the instance with the passed parameters %s',
+        extraParams => {
+          const params = {
             el: editorEl,
-            blobPath: undefined,
-            blobContent: undefined,
-          },
-        ]);
-
-        utils.initEditorLite({ el: editorEl, blobPath, blobContent });
-        expect(mockCreateMonacoInstance.mock.calls[1]).toEqual([
-          {
-            el: editorEl,
-            blobPath,
-            blobContent,
-          },
-        ]);
-      });
+            ...extraParams,
+          };
+
+          expect(Editor.prototype.createInstance).not.toHaveBeenCalled();
+
+          utils.initEditorLite(params);
+
+          expect(Editor.prototype.createInstance).toHaveBeenCalledWith(params);
+        },
+      );
     });
   });
 });

spec/frontend/editor/editor_lite_spec.js

@@ -2,13 +2,15 @@ import { editor as monacoEditor, languages as monacoLanguages, Uri } from 'monac
 import Editor from '~/editor/editor_lite';
 import { DEFAULT_THEME, themes } from '~/ide/lib/themes';
 
+const URI_PREFIX = 'gitlab';
+
 describe('Base editor', () => {
   let editorEl;
   let editor;
   const blobContent = 'Foo Bar';
   const blobPath = 'test.md';
-  const uri = new Uri('gitlab', false, blobPath);
-  const fakeModel = { foo: 'bar' };
+  const blobGlobalId = 'snippet_777';
+  const fakeModel = { foo: 'bar', dispose: jest.fn() };
 
   beforeEach(() => {
     setFixtures('<div id="editor" data-editor-loading></div>');
@@ -21,6 +23,8 @@ describe('Base editor', () => {
     editorEl.remove();
   });
 
+  const createUri = (...paths) => Uri.file([URI_PREFIX, ...paths].join('/'));
+
   it('initializes Editor with basic properties', () => {
     expect(editor).toBeDefined();
     expect(editor.editorEl).toBe(null);
@@ -65,7 +69,7 @@ describe('Base editor', () => {
     it('creates model to be supplied to Monaco editor', () => {
       editor.createInstance({ el: editorEl, blobPath, blobContent });
 
-      expect(modelSpy).toHaveBeenCalledWith(blobContent, undefined, uri);
+      expect(modelSpy).toHaveBeenCalledWith(blobContent, undefined, createUri(blobPath));
       expect(setModel).toHaveBeenCalledWith(fakeModel);
     });
 
@@ -75,6 +79,16 @@ describe('Base editor', () => {
       expect(editor.editorEl).not.toBe(null);
       expect(instanceSpy).toHaveBeenCalledWith(editorEl, expect.anything());
     });
+
+    it('with blobGlobalId, creates model with id in uri', () => {
+      editor.createInstance({ el: editorEl, blobPath, blobContent, blobGlobalId });
+
+      expect(modelSpy).toHaveBeenCalledWith(
+        blobContent,
+        undefined,
+        createUri(blobGlobalId, blobPath),
+      );
+    });
   });
 
   describe('implementation', () => {
@@ -82,10 +96,6 @@ describe('Base editor', () => {
       editor.createInstance({ el: editorEl, blobPath, blobContent });
     });
 
-    afterEach(() => {
-      editor.model.dispose();
-    });
-
     it('correctly proxies value from the model', () => {
       expect(editor.getValue()).toEqual(blobContent);
     });
@@ -132,10 +142,6 @@ describe('Base editor', () => {
       editor.createInstance({ el: editorEl, blobPath, blobContent });
     });
 
-    afterEach(() => {
-      editor.model.dispose();
-    });
-
     it('is extensible with the extensions', () => {
       expect(editor.foo).toBeUndefined();
 

spec/frontend/snippets/components/__snapshots__/snippet_blob_edit_spec.js.snap

@@ -17,6 +17,7 @@ exports[`Snippet Blob Edit component rendering matches the snapshot 1`] = `
     />
      
     <blob-content-edit-stub
+      fileglobalid="0a3d"
       filename="lorem.txt"
       value="Lorem ipsum dolor sit amet, consectetur adipiscing elit."
     />

spec/frontend/snippets/components/snippet_blob_edit_spec.js

@@ -51,6 +51,10 @@ describe('Snippet Blob Edit component', () => {
   }
 
   beforeEach(() => {
+    // This component generates a random id. Soon this will be abstracted away, but for now let's make this deterministic.
+    // see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38855
+    jest.spyOn(Math, 'random').mockReturnValue(0.04);
+
     axiosMock = new AxiosMockAdapter(axios);
     createComponent();
   });
@@ -68,7 +72,11 @@ describe('Snippet Blob Edit component', () => {
 
     it('renders required components', () => {
       expect(findComponent(BlobHeaderEdit).exists()).toBe(true);
-      expect(findComponent(BlobContentEdit).exists()).toBe(true);
+      expect(findComponent(BlobContentEdit).props()).toEqual({
+        fileGlobalId: expect.any(String),
+        fileName: '',
+        value: '',
+      });
     });
 
     it('renders loader if existing blob is supplied but no content is fetched yet', () => {