Merge branch '241025-remove-proxy-methods' into 'master'

Resolve "Remove proxy methods from Editor Lite's core"

See merge request !40662

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

@@ -46,7 +46,7 @@ export default {
       blobGlobalId: this.fileGlobalId,
     });
 
-    this.editor.onChangeContent(debounce(this.onFileChange.bind(this), 250));
+    this.editor.onDidChangeModelContent(debounce(this.onFileChange.bind(this), 250));
 
     window.requestAnimationFrame(() => {
       if (!performance.getEntriesByName(SNIPPET_MARK_BLOBS_CONTENT).length) {

app/assets/javascripts/blob/file_template_mediator.js

 import $ from 'jquery';
+
 import Api from '~/api';
+import toast from '~/vue_shared/plugins/global_toast';
+import { __ } from '~/locale';
+import initPopover from '~/blob/suggest_gitlab_ci_yml';
 
 import { deprecatedCreateFlash as Flash } from '../flash';
+
 import FileTemplateTypeSelector from './template_selectors/type_selector';
 import BlobCiYamlSelector from './template_selectors/ci_yaml_selector';
 import DockerfileSelector from './template_selectors/dockerfile_selector';
 import GitignoreSelector from './template_selectors/gitignore_selector';
 import LicenseSelector from './template_selectors/license_selector';
 import MetricsDashboardSelector from './template_selectors/metrics_dashboard_selector';
-import toast from '~/vue_shared/plugins/global_toast';
-import { __ } from '~/locale';
-import initPopover from '~/blob/suggest_gitlab_ci_yml';
 
 export default class FileTemplateMediator {
   constructor({ editor, currentAction, projectId }) {

app/assets/javascripts/blob/utils.js

@@ -6,12 +6,11 @@ export function initEditorLite({ el, ...args }) {
       alwaysConsumeMouseWheel: false,
     },
   });
-  editor.createInstance({
+
+  return editor.createInstance({
     el,
     ...args,
   });
-
-  return editor;
 }
 
 export default () => ({});

app/assets/javascripts/blob_edit/edit_blob.js

@@ -40,9 +40,10 @@ export default class EditBlob {
     const MarkdownExtensionPromise = this.options.isMarkdown
       ? import('~/editor/editor_markdown_ext')
       : Promise.resolve(false);
+    const FileTemplateExtensionPromise = import('~/editor/editor_file_template_ext');
 
-    return Promise.all([EditorPromise, MarkdownExtensionPromise])
-      .then(([EditorModule, MarkdownExtension]) => {
+    return Promise.all([EditorPromise, MarkdownExtensionPromise, FileTemplateExtensionPromise])
+      .then(([EditorModule, MarkdownExtension, FileTemplateExtension]) => {
         const EditorLite = EditorModule.default;
         const editorEl = document.getElementById('editor');
         const fileNameEl =
@@ -50,18 +51,16 @@ export default class EditBlob {
         const fileContentEl = document.getElementById('file-content');
         const form = document.querySelector('.js-edit-blob-form');
 
-        this.editor = new EditorLite();
+        const rootEditor = new EditorLite();
 
-        if (MarkdownExtension) {
-          this.editor.use(MarkdownExtension.default);
-        }
-
-        this.editor.createInstance({
+        this.editor = rootEditor.createInstance({
           el: editorEl,
           blobPath: fileNameEl.value,
           blobContent: editorEl.innerText,
         });
 
+        rootEditor.use([MarkdownExtension.default, FileTemplateExtension.default], this.editor);
+
         fileNameEl.addEventListener('change', () => {
           this.editor.updateModelLanguage(fileNameEl.value);
         });

app/assets/javascripts/editor/editor_file_template_ext.js

+import { Position } from 'monaco-editor';
+
+export default {
+  navigateFileStart() {
+    this.setPosition(new Position(1, 1));
+  },
+};

app/assets/javascripts/editor/editor_lite.js

-import { editor as monacoEditor, languages as monacoLanguages, Position, Uri } from 'monaco-editor';
+import { editor as monacoEditor, languages as monacoLanguages, Uri } from 'monaco-editor';
 import { DEFAULT_THEME, themes } from '~/ide/lib/themes';
 import languages from '~/ide/lib/languages';
 import { defaultEditorOptions } from '~/ide/lib/editor_options';
@@ -73,6 +73,7 @@ export default class Editor {
       this.instances.splice(index, 1);
       model.dispose();
     });
+    instance.updateModelLanguage = path => this.updateModelLanguage(path);
 
     // Reference to the model on the editor level will go away in
     // https://gitlab.com/gitlab-org/gitlab/-/issues/241023
@@ -92,10 +93,6 @@ 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;
@@ -107,46 +104,6 @@ export default class Editor {
     monacoEditor.setModelLanguage(this.model, id);
   }
 
-  /**
-   * @deprecated do not use .getValue() directly on the editor.
-   * This proxy-method will be removed in https://gitlab.com/gitlab-org/gitlab/-/issues/241025
-   * Rather use it on the exact instance
-   */
-  getValue() {
-    return this.instances[0].getValue();
-  }
-
-  /**
-   * @deprecated do not use .setValue() directly on the editor.
-   * This proxy-method will be removed in https://gitlab.com/gitlab-org/gitlab/-/issues/241025
-   * Rather use it on the exact instance
-   */
-  setValue(val) {
-    this.instances[0].setValue(val);
-  }
-
-  /**
-   * @deprecated do not use .focus() directly on the editor.
-   * This proxy-method will be removed in https://gitlab.com/gitlab-org/gitlab/-/issues/241025
-   * Rather use it on the exact instance
-   */
-  focus() {
-    this.instances[0].focus();
-  }
-
-  /**
-   * @deprecated do not use .updateOptions() directly on the editor.
-   * This proxy-method will be removed in https://gitlab.com/gitlab-org/gitlab/-/issues/241025
-   * Rather use it on the exact instance
-   */
-  updateOptions(options = {}) {
-    this.instances[0].updateOptions(options);
-  }
-
-  navigateFileStart() {
-    this.instances[0].setPosition(new Position(1, 1));
-  }
-
   use(exts = [], instance = null) {
     const extensions = Array.isArray(exts) ? exts : [exts];
     if (instance) {

spec/frontend/blob/components/blob_edit_content_spec.js

@@ -2,12 +2,14 @@ import { shallowMount } from '@vue/test-utils';
 import { nextTick } from 'vue';
 import BlobEditContent from '~/blob/components/blob_edit_content.vue';
 import * as utils from '~/blob/utils';
-import Editor from '~/editor/editor_lite';
 
 jest.mock('~/editor/editor_lite');
 
 describe('Blob Header Editing', () => {
   let wrapper;
+  const onDidChangeModelContent = jest.fn();
+  const updateModelLanguage = jest.fn();
+  const getValue = jest.fn();
   const value = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.';
   const fileName = 'lorem.txt';
   const fileGlobalId = 'snippet_777';
@@ -24,7 +26,12 @@ describe('Blob Header Editing', () => {
   }
 
   beforeEach(() => {
-    jest.spyOn(utils, 'initEditorLite');
+    jest.spyOn(utils, 'initEditorLite').mockImplementation(() => ({
+      onDidChangeModelContent,
+      updateModelLanguage,
+      getValue,
+      dispose: jest.fn(),
+    }));
 
     createComponent();
   });
@@ -34,8 +41,8 @@ describe('Blob Header Editing', () => {
   });
 
   const triggerChangeContent = val => {
-    jest.spyOn(Editor.prototype, 'getValue').mockReturnValue(val);
-    const [cb] = Editor.prototype.onChangeContent.mock.calls[0];
+    getValue.mockReturnValue(val);
+    const [cb] = onDidChangeModelContent.mock.calls[0];
 
     cb();
 
@@ -79,12 +86,12 @@ describe('Blob Header Editing', () => {
       });
 
       return nextTick().then(() => {
-        expect(Editor.prototype.updateModelLanguage).toHaveBeenCalledWith(newFileName);
+        expect(updateModelLanguage).toHaveBeenCalledWith(newFileName);
       });
     });
 
     it('registers callback with editor onChangeContent', () => {
-      expect(Editor.prototype.onChangeContent).toHaveBeenCalledWith(expect.any(Function));
+      expect(onDidChangeModelContent).toHaveBeenCalledWith(expect.any(Function));
     });
 
     it('emits input event when the blob content is changed', () => {

spec/frontend/blob_edit/edit_blob_spec.js

 import EditBlob from '~/blob_edit/edit_blob';
 import EditorLite from '~/editor/editor_lite';
 import MarkdownExtension from '~/editor/editor_markdown_ext';
+import FileTemplateExtension from '~/editor/editor_file_template_ext';
 
 jest.mock('~/editor/editor_lite');
 jest.mock('~/editor/editor_markdown_ext');
 
 describe('Blob Editing', () => {
+  const mockInstance = 'foo';
   beforeEach(() => {
     setFixtures(
-      `<div class="js-edit-blob-form"><div id="file_path"></div><div id="iditor"></div><input id="file-content"></div>`,
+      `<div class="js-edit-blob-form"><div id="file_path"></div><div id="editor"></div><input id="file-content"></div>`,
     );
+    jest.spyOn(EditorLite.prototype, 'createInstance').mockReturnValue(mockInstance);
   });
 
   const initEditor = (isMarkdown = false) => {
@@ -19,13 +22,29 @@ describe('Blob Editing', () => {
     });
   };
 
-  it('does not load MarkdownExtension by default', async () => {
+  it('loads FileTemplateExtension by default', async () => {
     await initEditor();
-    expect(EditorLite.prototype.use).not.toHaveBeenCalled();
+    expect(EditorLite.prototype.use).toHaveBeenCalledWith(
+      expect.arrayContaining([FileTemplateExtension]),
+      mockInstance,
+    );
   });
 
-  it('loads MarkdownExtension only for the markdown files', async () => {
-    await initEditor(true);
-    expect(EditorLite.prototype.use).toHaveBeenCalledWith(MarkdownExtension);
+  describe('Markdown', () => {
+    it('does not load MarkdownExtension by default', async () => {
+      await initEditor();
+      expect(EditorLite.prototype.use).not.toHaveBeenCalledWith(
+        expect.arrayContaining([MarkdownExtension]),
+        mockInstance,
+      );
+    });
+
+    it('loads MarkdownExtension only for the markdown files', async () => {
+      await initEditor(true);
+      expect(EditorLite.prototype.use).toHaveBeenCalledWith(
+        [MarkdownExtension, FileTemplateExtension],
+        mockInstance,
+      );
+    });
   });
 });

spec/frontend/editor/editor_markdown_ext_spec.js

@@ -78,18 +78,18 @@ describe('Markdown Extension for Editor Lite', () => {
       selectSecondString();
       instance.replaceSelectedText(expectedStr);
 
-      expect(editor.getValue()).toBe(`${firstLine}\n${expectedStr}\n${thirdLine}`);
+      expect(instance.getValue()).toBe(`${firstLine}\n${expectedStr}\n${thirdLine}`);
     });
 
     it('prepends the supplied text if no text is selected', () => {
       instance.replaceSelectedText(expectedStr);
-      expect(editor.getValue()).toBe(`${expectedStr}${firstLine}\n${secondLine}\n${thirdLine}`);
+      expect(instance.getValue()).toBe(`${expectedStr}${firstLine}\n${secondLine}\n${thirdLine}`);
     });
 
     it('replaces selection with empty string if no text is supplied', () => {
       selectSecondString();
       instance.replaceSelectedText();
-      expect(editor.getValue()).toBe(`${firstLine}\n\n${thirdLine}`);
+      expect(instance.getValue()).toBe(`${firstLine}\n\n${thirdLine}`);
     });
 
     it('puts cursor at the end of the new string and collapses selection by default', () => {

spec/frontend/snippet/snippet_bundle_spec.js

@@ -15,11 +15,13 @@ describe('Snippet editor', () => {
   const updatedMockContent = 'New Foo Bar';
 
   const mockEditor = {
-    createInstance: jest.fn(),
     updateModelLanguage: jest.fn(),
     getValue: jest.fn().mockReturnValueOnce(updatedMockContent),
   };
-  Editor.mockImplementation(() => mockEditor);
+  const createInstance = jest.fn().mockImplementation(() => ({ ...mockEditor }));
+  Editor.mockImplementation(() => ({
+    createInstance,
+  }));
 
   function setUpFixture(name, content) {
     setHTMLFixture(`
@@ -56,7 +58,7 @@ describe('Snippet editor', () => {
   });
 
   it('correctly initializes Editor', () => {
-    expect(mockEditor.createInstance).toHaveBeenCalledWith({
+    expect(createInstance).toHaveBeenCalledWith({
       el: editorEl,
       blobPath: mockName,
       blobContent: mockContent,