Commit 56d4bba5 authored by Vitaly Slobodin's avatar Vitaly Slobodin

Merge branch 'dmishunov-editor-lite-arhitecture' into 'master'

Refactoring Editor Lite's architecture

See merge request gitlab-org/gitlab!40257
parents 00cd763a e86358b2
import Editor from '~/editor/editor_lite';
export function initEditorLite({ el, ...args }) {
if (!el) {
throw new Error(`"el" parameter is required to initialize Editor`);
}
const editor = new Editor({
scrollbar: {
alwaysConsumeMouseWheel: false,
......
import { __ } from '~/locale';
export const EDITOR_LITE_INSTANCE_ERROR_NO_EL = __(
'"el" parameter is required for createInstance()',
);
export const URI_PREFIX = 'gitlab';
......@@ -5,13 +5,14 @@ import { defaultEditorOptions } from '~/ide/lib/editor_options';
import { registerLanguages } from '~/ide/utils';
import { joinPaths } from '~/lib/utils/url_utility';
import { clearDomElement } from './utils';
import { EDITOR_LITE_INSTANCE_ERROR_NO_EL, URI_PREFIX } from './constants';
export default class Editor {
constructor(options = {}) {
this.editorEl = null;
this.blobContent = '';
this.blobPath = '';
this.instance = null;
this.instances = [];
this.model = null;
this.options = {
extraEditorClassName: 'gl-editor-lite',
......@@ -40,31 +41,51 @@ export default class Editor {
* @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;
createInstance({
el = undefined,
blobPath = '',
blobContent = '',
blobGlobalId = '',
...instanceOptions
} = {}) {
if (!el) {
throw new Error(EDITOR_LITE_INSTANCE_ERROR_NO_EL);
}
this.editorEl = el;
this.blobContent = blobContent;
this.blobPath = blobPath;
clearDomElement(this.editorEl);
const uriFilePath = joinPaths('gitlab', blobGlobalId, blobPath);
const uriFilePath = joinPaths(URI_PREFIX, blobGlobalId, blobPath);
this.model = monacoEditor.createModel(this.blobContent, undefined, Uri.file(uriFilePath));
const model = monacoEditor.createModel(this.blobContent, undefined, Uri.file(uriFilePath));
monacoEditor.onDidCreateEditor(this.renderEditor.bind(this));
this.instance = monacoEditor.create(this.editorEl, this.options);
this.instance.setModel(this.model);
const instance = monacoEditor.create(this.editorEl, {
...this.options,
...instanceOptions,
});
instance.setModel(model);
instance.onDidDispose(() => {
const index = this.instances.findIndex(inst => inst === instance);
this.instances.splice(index, 1);
model.dispose();
});
// Reference to the model on the editor level will go away in
// https://gitlab.com/gitlab-org/gitlab/-/issues/241023
// After that, the references to the model will be routed through
// instance exclusively
this.model = model;
this.instances.push(instance);
return instance;
}
dispose() {
if (this.model) {
this.model.dispose();
this.model = null;
}
return this.instance && this.instance.dispose();
this.instances.forEach(instance => instance.dispose());
}
renderEditor() {
......@@ -86,28 +107,52 @@ 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.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.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.instance.focus();
this.instances[0].focus();
}
navigateFileStart() {
this.instance.setPosition(new Position(1, 1));
/**
* @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);
}
updateOptions(options = {}) {
this.instance.updateOptions(options);
navigateFileStart() {
this.instances[0].setPosition(new Position(1, 1));
}
use(exts = []) {
use(exts = [], instance = null) {
const extensions = Array.isArray(exts) ? exts : [exts];
Object.assign(this, ...extensions);
if (instance) {
Object.assign(instance, ...extensions);
} else {
this.instances.forEach(inst => Object.assign(inst, ...extensions));
}
}
}
export default {
getSelectedText(selection = this.getSelection()) {
const { startLineNumber, endLineNumber, startColumn, endColumn } = selection;
const valArray = this.instance.getValue().split('\n');
const valArray = this.getValue().split('\n');
let text = '';
if (startLineNumber === endLineNumber) {
text = valArray[startLineNumber - 1].slice(startColumn - 1, endColumn - 1);
......@@ -20,20 +20,16 @@ export default {
return text;
},
getSelection() {
return this.instance.getSelection();
},
replaceSelectedText(text, select = undefined) {
const forceMoveMarkers = !select;
this.instance.executeEdits('', [{ range: this.getSelection(), text, forceMoveMarkers }]);
this.executeEdits('', [{ range: this.getSelection(), text, forceMoveMarkers }]);
},
moveCursor(dx = 0, dy = 0) {
const pos = this.instance.getPosition();
const pos = this.getPosition();
pos.column += dx;
pos.lineNumber += dy;
this.instance.setPosition(pos);
this.setPosition(pos);
},
/**
......@@ -94,6 +90,6 @@ export default {
.setStartPosition(newStartLineNumber, newStartColumn)
.setEndPosition(newEndLineNumber, newEndColumn);
this.instance.setSelection(newSelection);
this.setSelection(newSelection);
},
};
......@@ -71,6 +71,9 @@ msgstr ""
msgid "\"%{path}\" did not exist on \"%{ref}\""
msgstr ""
msgid "\"el\" parameter is required for createInstance()"
msgstr ""
msgid "%d Scanned URL"
msgid_plural "%d Scanned URLs"
msgstr[0] ""
......
import { editor as monacoEditor, languages as monacoLanguages, Uri } from 'monaco-editor';
import Editor from '~/editor/editor_lite';
import { DEFAULT_THEME, themes } from '~/ide/lib/themes';
const URI_PREFIX = 'gitlab';
import { EDITOR_LITE_INSTANCE_ERROR_NO_EL, URI_PREFIX } from '~/editor/constants';
describe('Base editor', () => {
let editorEl;
......@@ -28,8 +27,8 @@ describe('Base editor', () => {
it('initializes Editor with basic properties', () => {
expect(editor).toBeDefined();
expect(editor.editorEl).toBe(null);
expect(editor.blobContent).toEqual('');
expect(editor.blobPath).toEqual('');
expect(editor.blobContent).toBe('');
expect(editor.blobPath).toBe('');
});
it('removes `editor-loading` data attribute from the target DOM element', () => {
......@@ -51,15 +50,18 @@ describe('Base editor', () => {
instanceSpy = jest.spyOn(monacoEditor, 'create').mockImplementation(() => ({
setModel,
dispose,
onDidDispose: jest.fn(),
}));
});
it('does nothing if no dom element is supplied', () => {
editor.createInstance();
it('throws an error if no dom element is supplied', () => {
expect(() => {
editor.createInstance();
}).toThrow(EDITOR_LITE_INSTANCE_ERROR_NO_EL);
expect(editor.editorEl).toBe(null);
expect(editor.blobContent).toEqual('');
expect(editor.blobPath).toEqual('');
expect(editor.blobContent).toBe('');
expect(editor.blobPath).toBe('');
expect(modelSpy).not.toHaveBeenCalled();
expect(instanceSpy).not.toHaveBeenCalled();
......@@ -89,15 +91,123 @@ describe('Base editor', () => {
createUri(blobGlobalId, blobPath),
);
});
it('initializes instance with passed properties', () => {
editor.createInstance({
el: editorEl,
blobContent,
blobPath,
});
expect(editor.editorEl).toBe(editorEl);
expect(editor.blobContent).toBe(blobContent);
expect(editor.blobPath).toBe(blobPath);
});
it('disposes instance when the editor is disposed', () => {
editor.createInstance({ el: editorEl, blobPath, blobContent, blobGlobalId });
expect(dispose).not.toHaveBeenCalled();
editor.dispose();
expect(dispose).toHaveBeenCalled();
});
});
describe('multiple instances', () => {
let instanceSpy;
let inst1Args;
let inst2Args;
let editorEl1;
let editorEl2;
let inst1;
let inst2;
const readOnlyIndex = '68'; // readOnly option has the internal index of 68 in the editor's options
beforeEach(() => {
setFixtures('<div id="editor1"></div><div id="editor2"></div>');
editorEl1 = document.getElementById('editor1');
editorEl2 = document.getElementById('editor2');
inst1Args = {
el: editorEl1,
blobGlobalId,
};
inst2Args = {
el: editorEl2,
blobContent,
blobPath,
blobGlobalId,
};
editor = new Editor();
instanceSpy = jest.spyOn(monacoEditor, 'create');
});
afterEach(() => {
editor.dispose();
});
it('can initialize several instances of the same editor', () => {
editor.createInstance(inst1Args);
expect(editor.editorEl).toBe(editorEl1);
expect(editor.instances).toHaveLength(1);
editor.createInstance(inst2Args);
expect(editor.editorEl).toBe(editorEl2);
expect(instanceSpy).toHaveBeenCalledTimes(2);
expect(editor.instances).toHaveLength(2);
});
it('shares global editor options among all instances', () => {
editor = new Editor({
readOnly: true,
});
inst1 = editor.createInstance(inst1Args);
expect(inst1.getOption(readOnlyIndex)).toBe(true);
inst2 = editor.createInstance(inst2Args);
expect(inst2.getOption(readOnlyIndex)).toBe(true);
});
it('allows overriding editor options on the instance level', () => {
editor = new Editor({
readOnly: true,
});
inst1 = editor.createInstance({
...inst1Args,
readOnly: false,
});
expect(inst1.getOption(readOnlyIndex)).toBe(false);
});
it('disposes instances and relevant models independently from each other', () => {
inst1 = editor.createInstance(inst1Args);
inst2 = editor.createInstance(inst2Args);
expect(inst1.getModel()).not.toBe(null);
expect(inst2.getModel()).not.toBe(null);
expect(editor.instances).toHaveLength(2);
expect(monacoEditor.getModels()).toHaveLength(2);
inst1.dispose();
expect(inst1.getModel()).toBe(null);
expect(inst2.getModel()).not.toBe(null);
expect(editor.instances).toHaveLength(1);
expect(monacoEditor.getModels()).toHaveLength(1);
});
});
describe('implementation', () => {
let instance;
beforeEach(() => {
editor.createInstance({ el: editorEl, blobPath, blobContent });
instance = editor.createInstance({ el: editorEl, blobPath, blobContent });
});
it('correctly proxies value from the model', () => {
expect(editor.getValue()).toEqual(blobContent);
expect(instance.getValue()).toBe(blobContent);
});
it('is capable of changing the language of the model', () => {
......@@ -108,10 +218,10 @@ describe('Base editor', () => {
const blobRenamedPath = 'test.js';
expect(editor.model.getLanguageIdentifier().language).toEqual('markdown');
expect(editor.model.getLanguageIdentifier().language).toBe('markdown');
editor.updateModelLanguage(blobRenamedPath);
expect(editor.model.getLanguageIdentifier().language).toEqual('javascript');
expect(editor.model.getLanguageIdentifier().language).toBe('javascript');
});
it('falls back to plaintext if there is no language associated with an extension', () => {
......@@ -121,11 +231,12 @@ describe('Base editor', () => {
editor.updateModelLanguage(blobRenamedPath);
expect(spy).not.toHaveBeenCalled();
expect(editor.model.getLanguageIdentifier().language).toEqual('plaintext');
expect(editor.model.getLanguageIdentifier().language).toBe('plaintext');
});
});
describe('extensions', () => {
let instance;
const foo1 = jest.fn();
const foo2 = jest.fn();
const bar = jest.fn();
......@@ -139,14 +250,14 @@ describe('Base editor', () => {
foo: foo2,
};
beforeEach(() => {
editor.createInstance({ el: editorEl, blobPath, blobContent });
instance = editor.createInstance({ el: editorEl, blobPath, blobContent });
});
it('is extensible with the extensions', () => {
expect(editor.foo).toBeUndefined();
expect(instance.foo).toBeUndefined();
editor.use(MyExt1);
expect(editor.foo).toEqual(foo1);
expect(instance.foo).toEqual(foo1);
});
it('does not fail if no extensions supplied', () => {
......@@ -157,18 +268,18 @@ describe('Base editor', () => {
});
it('is extensible with multiple extensions', () => {
expect(editor.foo).toBeUndefined();
expect(editor.bar).toBeUndefined();
expect(instance.foo).toBeUndefined();
expect(instance.bar).toBeUndefined();
editor.use([MyExt1, MyExt2]);
expect(editor.foo).toEqual(foo1);
expect(editor.bar).toEqual(bar);
expect(instance.foo).toEqual(foo1);
expect(instance.bar).toEqual(bar);
});
it('uses the last definition of a method in case of an overlap', () => {
editor.use([MyExt1, MyExt2, MyExt3]);
expect(editor).toEqual(
expect(instance).toEqual(
expect.objectContaining({
foo: foo2,
bar,
......@@ -179,15 +290,48 @@ describe('Base editor', () => {
it('correctly resolves references withing extensions', () => {
const FunctionExt = {
inst() {
return this.instance;
return this;
},
mod() {
return this.model;
return this.getModel();
},
};
editor.use(FunctionExt);
expect(editor.inst()).toEqual(editor.instance);
expect(editor.mod()).toEqual(editor.model);
expect(instance.inst()).toEqual(editor.instances[0]);
expect(instance.mod()).toEqual(editor.model);
});
describe('multiple instances', () => {
let inst1;
let inst2;
let editorEl1;
let editorEl2;
beforeEach(() => {
setFixtures('<div id="editor1"></div><div id="editor2"></div>');
editorEl1 = document.getElementById('editor1');
editorEl2 = document.getElementById('editor2');
inst1 = editor.createInstance({ el: editorEl1, blobPath: `foo-${blobPath}` });
inst2 = editor.createInstance({ el: editorEl2, blobPath: `bar-${blobPath}` });
});
afterEach(() => {
editor.dispose();
editorEl1.remove();
editorEl2.remove();
});
it('extends all instances if no specific instance is passed', () => {
editor.use(MyExt1);
expect(inst1.foo).toEqual(foo1);
expect(inst2.foo).toEqual(foo1);
});
it('extends specific instance if it has been passed', () => {
editor.use(MyExt1, inst2);
expect(inst1.foo).toBeUndefined();
expect(inst2.foo).toEqual(foo1);
});
});
});
......
......@@ -4,6 +4,7 @@ import EditorMarkdownExtension from '~/editor/editor_markdown_ext';
describe('Markdown Extension for Editor Lite', () => {
let editor;
let instance;
let editorEl;
const firstLine = 'This is a';
const secondLine = 'multiline';
......@@ -13,19 +14,19 @@ describe('Markdown Extension for Editor Lite', () => {
const setSelection = (startLineNumber = 1, startColumn = 1, endLineNumber = 1, endColumn = 1) => {
const selection = new Range(startLineNumber, startColumn, endLineNumber, endColumn);
editor.instance.setSelection(selection);
instance.setSelection(selection);
};
const selectSecondString = () => setSelection(2, 1, 2, secondLine.length + 1); // select the whole second line
const selectSecondAndThirdLines = () => setSelection(2, 1, 3, thirdLine.length + 1); // select second and third lines
const selectionToString = () => editor.instance.getSelection().toString();
const positionToString = () => editor.instance.getPosition().toString();
const selectionToString = () => instance.getSelection().toString();
const positionToString = () => instance.getPosition().toString();
beforeEach(() => {
setFixtures('<div id="editor" data-editor-loading></div>');
editorEl = document.getElementById('editor');
editor = new EditorLite();
editor.createInstance({
instance = editor.createInstance({
el: editorEl,
blobPath: filePath,
blobContent: text,
......@@ -34,17 +35,16 @@ describe('Markdown Extension for Editor Lite', () => {
});
afterEach(() => {
editor.instance.dispose();
editor.model.dispose();
editorEl.remove();
});
describe('getSelectedText', () => {
it('does not fail if there is no selection and returns the empty string', () => {
jest.spyOn(editor.instance, 'getSelection');
const resText = editor.getSelectedText();
jest.spyOn(instance, 'getSelection');
const resText = instance.getSelectedText();
expect(editor.instance.getSelection).toHaveBeenCalled();
expect(instance.getSelection).toHaveBeenCalled();
expect(resText).toBe('');
});
......@@ -56,7 +56,7 @@ describe('Markdown Extension for Editor Lite', () => {
`('correctly returns selected text for $description', ({ selection, expectedString }) => {
setSelection(...selection);
const resText = editor.getSelectedText();
const resText = instance.getSelectedText();
expect(resText).toBe(expectedString);
});
......@@ -65,7 +65,7 @@ describe('Markdown Extension for Editor Lite', () => {
selectSecondString();
const firstLineSelection = new Range(1, 1, 1, firstLine.length + 1);
const resText = editor.getSelectedText(firstLineSelection);
const resText = instance.getSelectedText(firstLineSelection);
expect(resText).toBe(firstLine);
});
......@@ -76,25 +76,25 @@ describe('Markdown Extension for Editor Lite', () => {
it('replaces selected text with the supplied one', () => {
selectSecondString();
editor.replaceSelectedText(expectedStr);
instance.replaceSelectedText(expectedStr);
expect(editor.getValue()).toBe(`${firstLine}\n${expectedStr}\n${thirdLine}`);
});
it('prepends the supplied text if no text is selected', () => {
editor.replaceSelectedText(expectedStr);
instance.replaceSelectedText(expectedStr);
expect(editor.getValue()).toBe(`${expectedStr}${firstLine}\n${secondLine}\n${thirdLine}`);
});
it('replaces selection with empty string if no text is supplied', () => {
selectSecondString();
editor.replaceSelectedText();
instance.replaceSelectedText();
expect(editor.getValue()).toBe(`${firstLine}\n\n${thirdLine}`);
});
it('puts cursor at the end of the new string and collapses selection by default', () => {
selectSecondString();
editor.replaceSelectedText(expectedStr);
instance.replaceSelectedText(expectedStr);
expect(positionToString()).toBe(`(2,${expectedStr.length + 1})`);
expect(selectionToString()).toBe(
......@@ -106,7 +106,7 @@ describe('Markdown Extension for Editor Lite', () => {
const select = 'url';
const complexReplacementString = `[${secondLine}](${select})`;
selectSecondString();
editor.replaceSelectedText(complexReplacementString, select);
instance.replaceSelectedText(complexReplacementString, select);
expect(positionToString()).toBe(`(2,${complexReplacementString.length + 1})`);
expect(selectionToString()).toBe(`[2,1 -> 2,${complexReplacementString.length + 1}]`);
......@@ -116,7 +116,7 @@ describe('Markdown Extension for Editor Lite', () => {
describe('moveCursor', () => {
const setPosition = endCol => {
const currentPos = new Position(2, endCol);
editor.instance.setPosition(currentPos);
instance.setPosition(currentPos);
};
it.each`
......@@ -136,9 +136,9 @@ describe('Markdown Extension for Editor Lite', () => {
({ startColumn, shift, endPosition }) => {
setPosition(startColumn);
if (Array.isArray(shift)) {
editor.moveCursor(...shift);
instance.moveCursor(...shift);
} else {
editor.moveCursor(shift);
instance.moveCursor(shift);
}
expect(positionToString()).toBe(endPosition);
},
......@@ -153,18 +153,18 @@ describe('Markdown Extension for Editor Lite', () => {
expect(selectionToString()).toBe(`[2,1 -> 3,${thirdLine.length + 1}]`);
editor.selectWithinSelection(toSelect, selectedText);
instance.selectWithinSelection(toSelect, selectedText);
expect(selectionToString()).toBe(`[3,1 -> 3,${toSelect.length + 1}]`);
});
it('does not fail when only `toSelect` is supplied and fetches the text from selection', () => {
jest.spyOn(editor, 'getSelectedText');
jest.spyOn(instance, 'getSelectedText');
const toSelect = 'string';
selectSecondAndThirdLines();
editor.selectWithinSelection(toSelect);
instance.selectWithinSelection(toSelect);
expect(editor.getSelectedText).toHaveBeenCalled();
expect(instance.getSelectedText).toHaveBeenCalled();
expect(selectionToString()).toBe(`[3,1 -> 3,${toSelect.length + 1}]`);
});
......@@ -176,7 +176,7 @@ describe('Markdown Extension for Editor Lite', () => {
expect(positionToString()).toBe(expectedPos);
expect(selectionToString()).toBe(expectedSelection);
editor.selectWithinSelection();
instance.selectWithinSelection();
expect(positionToString()).toBe(expectedPos);
expect(selectionToString()).toBe(expectedSelection);
......@@ -190,12 +190,12 @@ describe('Markdown Extension for Editor Lite', () => {
expect(positionToString()).toBe(expectedPos);
expect(selectionToString()).toBe(expectedSelection);
editor.selectWithinSelection(toSelect);
instance.selectWithinSelection(toSelect);
expect(positionToString()).toBe(expectedPos);
expect(selectionToString()).toBe(expectedSelection);
editor.selectWithinSelection();
instance.selectWithinSelection();
expect(positionToString()).toBe(expectedPos);
expect(selectionToString()).toBe(expectedSelection);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment