diff --git a/app/assets/javascripts/ide/lib/files.js b/app/assets/javascripts/ide/lib/files.js new file mode 100644 index 00000000000..5dfba8fe531 --- /dev/null +++ b/app/assets/javascripts/ide/lib/files.js @@ -0,0 +1,113 @@ +import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils'; +import { decorateData, sortTree } from '../stores/utils'; + +export const splitParent = path => { + const idx = path.lastIndexOf('/'); + + return { + parent: idx >= 0 ? path.substring(0, idx) : null, + name: idx >= 0 ? path.substring(idx + 1) : path, + }; +}; + +/** + * Create file objects from a list of file paths. + */ +export const decorateFiles = ({ + data, + projectId, + branchId, + tempFile = false, + content = '', + base64 = false, +}) => { + const treeList = []; + const entries = {}; + + // These mutable variable references end up being exported and used by `createTempEntry` + let file; + let parentPath; + + const insertParent = path => { + if (!path) { + return null; + } else if (entries[path]) { + return entries[path]; + } + + const { parent, name } = splitParent(path); + const parentFolder = parent && insertParent(parent); + parentPath = parentFolder && parentFolder.path; + + const tree = decorateData({ + projectId, + branchId, + id: path, + name, + path, + url: `/${projectId}/tree/${branchId}/-/${path}/`, + type: 'tree', + parentTreeUrl: parentFolder ? parentFolder.url : `/${projectId}/tree/${branchId}/`, + tempFile, + changed: tempFile, + opened: tempFile, + parentPath, + }); + + Object.assign(entries, { + [path]: tree, + }); + + if (parentFolder) { + parentFolder.tree.push(tree); + } else { + treeList.push(tree); + } + + return tree; + }; + + data.forEach(path => { + const { parent, name } = splitParent(path); + + const fileFolder = parent && insertParent(parent); + + if (name) { + parentPath = fileFolder && fileFolder.path; + + file = decorateData({ + projectId, + branchId, + id: path, + name, + path, + url: `/${projectId}/blob/${branchId}/-/${path}`, + type: 'blob', + parentTreeUrl: fileFolder ? fileFolder.url : `/${projectId}/blob/${branchId}`, + tempFile, + changed: tempFile, + content, + base64, + previewMode: viewerInformationForPath(name), + parentPath, + }); + + Object.assign(entries, { + [path]: file, + }); + + if (fileFolder) { + fileFolder.tree.push(file); + } else { + treeList.push(file); + } + } + }); + + return { + entries, + treeList: sortTree(treeList), + file, + parentPath, + }; +}; diff --git a/app/assets/javascripts/ide/stores/actions.js b/app/assets/javascripts/ide/stores/actions.js index 95d91e08757..7b660bda081 100644 --- a/app/assets/javascripts/ide/stores/actions.js +++ b/app/assets/javascripts/ide/stores/actions.js @@ -3,7 +3,7 @@ import Vue from 'vue'; import { visitUrl } from '~/lib/utils/url_utility'; import flash from '~/flash'; import * as types from './mutation_types'; -import FilesDecoratorWorker from './workers/files_decorator_worker'; +import { decorateFiles } from '../lib/files'; import { stageKeys } from '../constants'; export const redirectToUrl = (_, url) => visitUrl(url); @@ -56,7 +56,6 @@ export const createTempEntry = ( { name, type, content = '', base64 = false }, ) => new Promise(resolve => { - const worker = new FilesDecoratorWorker(); const fullName = name.slice(-1) !== '/' && type === 'tree' ? `${name}/` : name; if (state.entries[name]) { @@ -74,31 +73,7 @@ export const createTempEntry = ( return null; } - worker.addEventListener('message', ({ data }) => { - const { file, parentPath } = data; - - worker.terminate(); - - commit(types.CREATE_TMP_ENTRY, { - data, - projectId: state.currentProjectId, - branchId: state.currentBranchId, - }); - - if (type === 'blob') { - commit(types.TOGGLE_FILE_OPEN, file.path); - commit(types.ADD_FILE_TO_CHANGED, file.path); - dispatch('setFileActive', file.path); - } - - if (parentPath && !state.entries[parentPath].opened) { - commit(types.TOGGLE_TREE_OPEN, parentPath); - } - - resolve(file); - }); - - worker.postMessage({ + const data = decorateFiles({ data: [fullName], projectId: state.currentProjectId, branchId: state.currentBranchId, @@ -107,6 +82,25 @@ export const createTempEntry = ( base64, content, }); + const { file, parentPath } = data; + + commit(types.CREATE_TMP_ENTRY, { + data, + projectId: state.currentProjectId, + branchId: state.currentBranchId, + }); + + if (type === 'blob') { + commit(types.TOGGLE_FILE_OPEN, file.path); + commit(types.ADD_FILE_TO_CHANGED, file.path); + dispatch('setFileActive', file.path); + } + + if (parentPath && !state.entries[parentPath].opened) { + commit(types.TOGGLE_TREE_OPEN, parentPath); + } + + resolve(file); return null; }); diff --git a/app/assets/javascripts/ide/stores/actions/tree.js b/app/assets/javascripts/ide/stores/actions/tree.js index de5f6050074..3d83e4a9ba5 100644 --- a/app/assets/javascripts/ide/stores/actions/tree.js +++ b/app/assets/javascripts/ide/stores/actions/tree.js @@ -1,7 +1,8 @@ +import _ from 'underscore'; import { __ } from '../../../locale'; import service from '../../services'; import * as types from '../mutation_types'; -import FilesDecoratorWorker from '../workers/files_decorator_worker'; +import { decorateFiles } from '../../lib/files'; export const toggleTreeOpen = ({ commit }, path) => { commit(types.TOGGLE_TREE_OPEN, path); @@ -32,6 +33,19 @@ export const handleTreeEntryAction = ({ commit, dispatch }, row) => { dispatch('showTreeEntry', row.path); }; +export const setDirectoryData = ({ state, commit }, { projectId, branchId, treeList }) => { + const selectedTree = state.trees[`${projectId}/${branchId}`]; + + commit(types.SET_DIRECTORY_DATA, { + treePath: `${projectId}/${branchId}`, + data: treeList, + }); + commit(types.TOGGLE_LOADING, { + entry: selectedTree, + forceValue: false, + }); +}; + export const getFiles = ({ state, commit, dispatch }, { projectId, branchId } = {}) => new Promise((resolve, reject) => { if ( @@ -45,31 +59,19 @@ export const getFiles = ({ state, commit, dispatch }, { projectId, branchId } = service .getFiles(selectedProject.web_url, branchId) .then(({ data }) => { - const worker = new FilesDecoratorWorker(); - worker.addEventListener('message', e => { - const { entries, treeList } = e.data; - const selectedTree = state.trees[`${projectId}/${branchId}`]; - - commit(types.SET_ENTRIES, entries); - commit(types.SET_DIRECTORY_DATA, { - treePath: `${projectId}/${branchId}`, - data: treeList, - }); - commit(types.TOGGLE_LOADING, { - entry: selectedTree, - forceValue: false, - }); - - worker.terminate(); - - resolve(); - }); - - worker.postMessage({ + const { entries, treeList } = decorateFiles({ data, projectId, branchId, }); + + commit(types.SET_ENTRIES, entries); + + // Defer setting the directory data because this triggers some intense rendering. + // The entries is all we need to load the file editor. + _.defer(() => dispatch('setDirectoryData', { projectId, branchId, treeList })); + + resolve(); }) .catch(e => { if (e.response.status === 404) { diff --git a/app/assets/javascripts/ide/stores/utils.js b/app/assets/javascripts/ide/stores/utils.js index 0ede76fd1e0..0b2a18e9c8a 100644 --- a/app/assets/javascripts/ide/stores/utils.js +++ b/app/assets/javascripts/ide/stores/utils.js @@ -75,8 +75,7 @@ export const decorateData = entity => { parentPath = '', } = entity; - return { - ...dataStructure(), + return Object.assign(dataStructure(), { id, projectId, branchId, @@ -97,7 +96,7 @@ export const decorateData = entity => { file_lock, html, parentPath, - }; + }); }; export const findEntry = (tree, type, name, prop = 'name') => diff --git a/app/assets/javascripts/ide/stores/workers/files_decorator_worker.js b/app/assets/javascripts/ide/stores/workers/files_decorator_worker.js deleted file mode 100644 index fa35c215880..00000000000 --- a/app/assets/javascripts/ide/stores/workers/files_decorator_worker.js +++ /dev/null @@ -1,100 +0,0 @@ -import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils'; -import { decorateData, sortTree } from '../utils'; - -// eslint-disable-next-line no-restricted-globals -self.addEventListener('message', e => { - const { data, projectId, branchId, tempFile = false, content = '', base64 = false } = e.data; - - const treeList = []; - let file; - let parentPath; - const entries = data.reduce((acc, path) => { - const pathSplit = path.split('/'); - const blobName = pathSplit.pop().trim(); - - if (pathSplit.length > 0) { - pathSplit.reduce((pathAcc, folderName) => { - const parentFolder = acc[pathAcc[pathAcc.length - 1]]; - const folderPath = `${parentFolder ? `${parentFolder.path}/` : ''}${folderName}`; - const foundEntry = acc[folderPath]; - - if (!foundEntry) { - parentPath = parentFolder ? parentFolder.path : null; - - const tree = decorateData({ - projectId, - branchId, - id: folderPath, - name: folderName, - path: folderPath, - url: `/${projectId}/tree/${branchId}/-/${folderPath}/`, - type: 'tree', - parentTreeUrl: parentFolder ? parentFolder.url : `/${projectId}/tree/${branchId}/`, - tempFile, - changed: tempFile, - opened: tempFile, - parentPath, - }); - - Object.assign(acc, { - [folderPath]: tree, - }); - - if (parentFolder) { - parentFolder.tree.push(tree); - } else { - treeList.push(tree); - } - - pathAcc.push(tree.path); - } else { - pathAcc.push(foundEntry.path); - } - - return pathAcc; - }, []); - } - - if (blobName !== '') { - const fileFolder = acc[pathSplit.join('/')]; - parentPath = fileFolder ? fileFolder.path : null; - - file = decorateData({ - projectId, - branchId, - id: path, - name: blobName, - path, - url: `/${projectId}/blob/${branchId}/-/${path}`, - type: 'blob', - parentTreeUrl: fileFolder ? fileFolder.url : `/${projectId}/blob/${branchId}`, - tempFile, - changed: tempFile, - content, - base64, - previewMode: viewerInformationForPath(blobName), - parentPath, - }); - - Object.assign(acc, { - [path]: file, - }); - - if (fileFolder) { - fileFolder.tree.push(file); - } else { - treeList.push(file); - } - } - - return acc; - }, {}); - - // eslint-disable-next-line no-restricted-globals - self.postMessage({ - entries, - treeList: sortTree(treeList), - file, - parentPath, - }); -}); diff --git a/changelogs/unreleased/53336-improve-web-ide-launch-performance.yml b/changelogs/unreleased/53336-improve-web-ide-launch-performance.yml new file mode 100644 index 00000000000..65439f5a6c2 --- /dev/null +++ b/changelogs/unreleased/53336-improve-web-ide-launch-performance.yml @@ -0,0 +1,5 @@ +--- +title: Improve Web IDE launch performance +merge_request: 25700 +author: +type: performance diff --git a/spec/javascripts/ide/lib/files_spec.js b/spec/javascripts/ide/lib/files_spec.js new file mode 100644 index 00000000000..fe791aa2b74 --- /dev/null +++ b/spec/javascripts/ide/lib/files_spec.js @@ -0,0 +1,77 @@ +import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils'; +import { decorateFiles, splitParent } from '~/ide/lib/files'; +import { decorateData } from '~/ide/stores/utils'; + +const TEST_BRANCH_ID = 'lorem-ipsum'; +const TEST_PROJECT_ID = 10; + +const createEntries = paths => { + const createEntry = (acc, { path, type, children }) => { + // Sometimes we need to end the url with a '/' + const createUrl = base => (type === 'tree' ? `${base}/` : base); + + const { name, parent } = splitParent(path); + const parentEntry = acc[parent]; + + acc[path] = { + ...decorateData({ + projectId: TEST_PROJECT_ID, + branchId: TEST_BRANCH_ID, + id: path, + name, + path, + url: createUrl(`/${TEST_PROJECT_ID}/${type}/${TEST_BRANCH_ID}/-/${path}`), + type, + previewMode: viewerInformationForPath(path), + parentPath: parent, + parentTreeUrl: parentEntry + ? parentEntry.url + : createUrl(`/${TEST_PROJECT_ID}/${type}/${TEST_BRANCH_ID}`), + }), + tree: children.map(childName => jasmine.objectContaining({ name: childName })), + }; + + return acc; + }; + + const entries = paths.reduce(createEntry, {}); + + // Wrap entries in jasmine.objectContaining. + // We couldn't do this earlier because we still need to select properties from parent entries. + return Object.keys(entries).reduce((acc, key) => { + acc[key] = jasmine.objectContaining(entries[key]); + + return acc; + }, {}); +}; + +describe('IDE lib decorate files', () => { + it('creates entries and treeList', () => { + const data = ['app/assets/apples/foo.js', 'app/bugs.js', 'README.md']; + const expectedEntries = createEntries([ + { path: 'app', type: 'tree', children: ['assets', 'bugs.js'] }, + { path: 'app/assets', type: 'tree', children: ['apples'] }, + { path: 'app/assets/apples', type: 'tree', children: ['foo.js'] }, + { path: 'app/assets/apples/foo.js', type: 'blob', children: [] }, + { path: 'app/bugs.js', type: 'blob', children: [] }, + { path: 'README.md', type: 'blob', children: [] }, + ]); + + const { entries, treeList } = decorateFiles({ + data, + branchId: TEST_BRANCH_ID, + projectId: TEST_PROJECT_ID, + }); + + // Here we test the keys and then each key/value individually because `expect(entries).toEqual(expectedEntries)` + // was taking a very long time for some reason. Probably due to large objects and nested `jasmine.objectContaining`. + const entryKeys = Object.keys(entries); + + expect(entryKeys).toEqual(Object.keys(expectedEntries)); + entryKeys.forEach(key => { + expect(entries[key]).toEqual(expectedEntries[key]); + }); + + expect(treeList).toEqual([expectedEntries.app, expectedEntries['README.md']]); + }); +}); diff --git a/spec/javascripts/ide/stores/actions/tree_spec.js b/spec/javascripts/ide/stores/actions/tree_spec.js index bd41e87bf0e..fbb676aab33 100644 --- a/spec/javascripts/ide/stores/actions/tree_spec.js +++ b/spec/javascripts/ide/stores/actions/tree_spec.js @@ -20,6 +20,7 @@ describe('Multi-file store tree actions', () => { }; beforeEach(() => { + jasmine.clock().install(); spyOn(router, 'push'); mock = new MockAdapter(axios); @@ -37,6 +38,7 @@ describe('Multi-file store tree actions', () => { }); afterEach(() => { + jasmine.clock().uninstall(); mock.restore(); resetStore(store); }); @@ -69,6 +71,11 @@ describe('Multi-file store tree actions', () => { it('adds data into tree', done => { store .dispatch('getFiles', basicCallParameters) + .then(() => { + // The populating of the tree is deferred for performance reasons. + // See this merge request for details: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/25700 + jasmine.clock().tick(1); + }) .then(() => { projectTree = store.state.trees['abcproject/master'];