Merge branch '53336-improve-web-ide-launch-performance' into 'master'

Resolve "Improve Web IDE launch performance"

Closes #53336

See merge request gitlab-org/gitlab-ce!25700

app/assets/javascripts/ide/lib/files.js

+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,
+  };
+};

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;
   });

app/assets/javascripts/ide/stores/actions/tree.js

+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) {

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') =>

app/assets/javascripts/ide/stores/workers/files_decorator_worker.js

-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,
-  });
-});

changelogs/unreleased/53336-improve-web-ide-launch-performance.yml

+---
+title: Improve Web IDE launch performance
+merge_request: 25700
+author:
+type: performance

spec/javascripts/ide/lib/files_spec.js

+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']]);
+  });
+});

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'];