Commit 17b84299 authored by Paul Slaughter's avatar Paul Slaughter Committed by Miguel Rincon

Use ide.files.change for live preview

Watching deep on entries causes way
too many updates and is expensive.
parent 413d2a37
<script>
import { mapActions, mapGetters, mapState } from 'vuex';
import { isEmpty } from 'lodash';
import { isEmpty, debounce } from 'lodash';
import { Manager } from 'smooshpack';
import { listen } from 'codesandbox-api';
import { GlLoadingIcon } from '@gitlab/ui';
import Navigator from './navigator.vue';
import { packageJsonPath } from '../../constants';
import { packageJsonPath, LIVE_PREVIEW_DEBOUNCE } from '../../constants';
import { createPathWithExt } from '../../utils';
import eventHub from '../../eventhub';
export default {
components: {
......@@ -61,13 +62,10 @@ export default {
};
},
},
watch: {
entries: {
deep: true,
handler: 'update',
},
},
mounted() {
this.onFilesChangeCallback = debounce(() => this.update(), LIVE_PREVIEW_DEBOUNCE);
eventHub.$on('ide.files.change', this.onFilesChangeCallback);
this.loading = true;
return this.loadFileContent(packageJsonPath)
......@@ -78,17 +76,19 @@ export default {
.then(() => this.initPreview());
},
beforeDestroy() {
// Setting sandpackReady = false protects us form a phantom `update()` being called when `debounce` finishes.
this.sandpackReady = false;
eventHub.$off('ide.files.change', this.onFilesChangeCallback);
if (!isEmpty(this.manager)) {
this.manager.listener();
}
this.manager = {};
if (this.listener) {
this.listener();
}
clearTimeout(this.timeout);
this.timeout = null;
},
methods: {
...mapActions(['getFileData', 'getRawFileData']),
......@@ -122,17 +122,13 @@ export default {
update() {
if (!this.sandpackReady) return;
clearTimeout(this.timeout);
this.timeout = setTimeout(() => {
if (isEmpty(this.manager)) {
this.initPreview();
if (isEmpty(this.manager)) {
this.initPreview();
return;
}
return;
}
this.manager.updatePreview(this.sandboxOpts);
}, 250);
this.manager.updatePreview(this.sandboxOpts);
},
initManager() {
const { codesandboxBundlerUrl: bundlerURL } = this;
......
......@@ -97,3 +97,6 @@ export const packageJsonPath = 'package.json';
export const SIDE_LEFT = 'left';
export const SIDE_RIGHT = 'right';
// Live Preview feature
export const LIVE_PREVIEW_DEBOUNCE = 2000;
......@@ -10,7 +10,7 @@ import eventHub from '../../eventhub';
import service from '../../services';
import * as types from '../mutation_types';
import { setPageTitleForFile } from '../utils';
import { viewerTypes, stageKeys } from '../../constants';
import { viewerTypes, stageKeys, commitActionTypes } from '../../constants';
export const closeFile = ({ commit, state, dispatch, getters }, file) => {
const { path } = file;
......@@ -164,7 +164,7 @@ export const getRawFileData = ({ state, commit, dispatch, getters }, { path }) =
});
};
export const changeFileContent = ({ commit, state, getters }, { path, content }) => {
export const changeFileContent = ({ commit, dispatch, state, getters }, { path, content }) => {
const file = state.entries[path];
// It's possible for monaco to hit a race condition where it tries to update renamed files.
......@@ -185,6 +185,8 @@ export const changeFileContent = ({ commit, state, getters }, { path, content })
} else if (!file.changed && !file.tempFile && indexOfChangedFile !== -1) {
commit(types.REMOVE_FILE_FROM_CHANGED, path);
}
dispatch('triggerFilesChange', { type: commitActionTypes.update, path });
};
export const restoreOriginalFile = ({ dispatch, state, commit }, path) => {
......
......@@ -9,10 +9,15 @@ const removeUnusedFileEditors = (store) => {
export const setupFileEditorsSync = (store) => {
eventHub.$on('ide.files.change', ({ type, ...payload } = {}) => {
// Do nothing on file update because the file tree itself hasn't changed.
if (type === commitActionTypes.update) {
return;
}
if (type === commitActionTypes.move) {
store.dispatch('editor/renameFileEditor', payload);
} else {
// The files have changed, but the specific change is not known.
// The file tree has changed, but the specific change is not known.
removeUnusedFileEditors(store);
}
});
......
import { debounce } from 'lodash';
import eventHub from '~/ide/eventhub';
import { commitActionTypes } from '~/ide/constants';
import terminalSyncModule from '../modules/terminal_sync';
import { isEndingStatus, isRunningStatus } from '../modules/terminal/utils';
......@@ -19,16 +20,25 @@ export default function createMirrorPlugin() {
store.dispatch(`terminalSync/upload`);
}, UPLOAD_DEBOUNCE);
const onFilesChange = (payload) => {
// Do nothing on a file update since we only want to trigger manually on "save".
if (payload?.type === commitActionTypes.update) {
return;
}
upload();
};
const stop = () => {
store.dispatch(`terminalSync/stop`);
eventHub.$off('ide.files.change', upload);
eventHub.$off('ide.files.change', onFilesChange);
};
const start = () => {
store
.dispatch(`terminalSync/start`)
.then(() => {
eventHub.$on('ide.files.change', upload);
eventHub.$on('ide.files.change', onFilesChange);
})
.catch(() => {
// error is handled in store
......
---
title: Fix over-eagerly updating Web IDE Live Preview
merge_request: 50255
author:
type: fixed
......@@ -3,6 +3,7 @@ import { GlLoadingIcon } from '@gitlab/ui';
import { shallowMount, createLocalVue } from '@vue/test-utils';
import smooshpack from 'smooshpack';
import Clientside from '~/ide/components/preview/clientside.vue';
import eventHub from '~/ide/eventhub';
jest.mock('smooshpack', () => ({
Manager: jest.fn(),
......@@ -70,6 +71,17 @@ describe('IDE clientside preview', () => {
});
};
const createInitializedComponent = () => {
createComponent();
wrapper.setData({
sandpackReady: true,
manager: {
listener: jest.fn(),
updatePreview: jest.fn(),
},
});
};
afterEach(() => {
wrapper.destroy();
});
......@@ -293,33 +305,33 @@ describe('IDE clientside preview', () => {
});
describe('update', () => {
beforeEach(() => {
createComponent();
wrapper.setData({ sandpackReady: true });
});
it('initializes manager if manager is empty', () => {
createComponent({ getters: { packageJson: dummyPackageJson } });
wrapper.setData({ sandpackReady: true });
wrapper.vm.update();
jest.advanceTimersByTime(250);
return waitForCalls().then(() => {
expect(smooshpack.Manager).toHaveBeenCalled();
});
});
it('calls updatePreview', () => {
wrapper.setData({
manager: {
listener: jest.fn(),
updatePreview: jest.fn(),
},
});
createInitializedComponent();
wrapper.vm.update();
jest.advanceTimersByTime(250);
expect(wrapper.vm.manager.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts);
});
});
describe('on ide.files.change event', () => {
beforeEach(() => {
createInitializedComponent();
eventHub.$emit('ide.files.change');
});
it('calls updatePreview', () => {
expect(wrapper.vm.manager.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts);
});
});
......@@ -355,4 +367,18 @@ describe('IDE clientside preview', () => {
});
});
});
describe('when destroyed', () => {
let spy;
beforeEach(() => {
createInitializedComponent();
spy = wrapper.vm.manager.updatePreview;
wrapper.destroy();
});
it('does not call updatePreview', () => {
expect(spy).not.toHaveBeenCalled();
});
});
});
......@@ -41,5 +41,10 @@ export const createTriggerRenamePayload = (path, newPath) => ({
newPath,
});
export const createTriggerUpdatePayload = (path) => ({
type: commitActionTypes.update,
path,
});
export const createTriggerRenameAction = (path, newPath) =>
createTriggerChangeAction(createTriggerRenamePayload(path, newPath));
......@@ -7,7 +7,7 @@ import * as types from '~/ide/stores/mutation_types';
import service from '~/ide/services';
import { createRouter } from '~/ide/ide_router';
import eventHub from '~/ide/eventhub';
import { file, createTriggerRenameAction } from '../../helpers';
import { file, createTriggerRenameAction, createTriggerUpdatePayload } from '../../helpers';
const ORIGINAL_CONTENT = 'original content';
const RELATIVE_URL_ROOT = '/gitlab';
......@@ -510,12 +510,15 @@ describe('IDE store file actions', () => {
describe('changeFileContent', () => {
let tmpFile;
let onFilesChange;
beforeEach(() => {
tmpFile = file('tmpFile');
tmpFile.content = '\n';
tmpFile.raw = '\n';
store.state.entries[tmpFile.path] = tmpFile;
onFilesChange = jest.fn();
eventHub.$on('ide.files.change', onFilesChange);
});
it('updates file content', () => {
......@@ -580,6 +583,17 @@ describe('IDE store file actions', () => {
expect(store.state.changedFiles.length).toBe(0);
});
});
it('triggers ide.files.change', async () => {
expect(onFilesChange).not.toHaveBeenCalled();
await store.dispatch('changeFileContent', {
path: tmpFile.path,
content: 'content\n',
});
expect(onFilesChange).toHaveBeenCalledWith(createTriggerUpdatePayload(tmpFile.path));
});
});
describe('with changed file', () => {
......
import { cloneDeep } from 'lodash';
import Vuex from 'vuex';
import eventHub from '~/ide/eventhub';
import { createStoreOptions } from '~/ide/stores';
import { setupFileEditorsSync } from '~/ide/stores/modules/editor/setup';
import { createTriggerRenamePayload } from '../../../helpers';
import { createTriggerRenamePayload, createTriggerUpdatePayload } from '../../../helpers';
describe('~/ide/stores/modules/editor/setup', () => {
let store;
......@@ -33,6 +34,14 @@ describe('~/ide/stores/modules/editor/setup', () => {
});
});
it('when files update is emitted, does nothing', () => {
const origState = cloneDeep(store.state);
eventHub.$emit('ide.files.change', createTriggerUpdatePayload('foo'));
expect(store.state).toEqual(origState);
});
it('when files rename is emitted, renames fileEditor', () => {
eventHub.$emit('ide.files.change', createTriggerRenamePayload('foo', 'foo_new'));
......
......@@ -4,6 +4,7 @@ import { SET_SESSION_STATUS } from '~/ide/stores/modules/terminal/mutation_types
import { RUNNING, STOPPING } from '~/ide/stores/modules/terminal/constants';
import { createStore } from '~/ide/stores';
import eventHub from '~/ide/eventhub';
import { createTriggerUpdatePayload } from '../../helpers';
jest.mock('~/ide/lib/mirror');
......@@ -51,6 +52,14 @@ describe('IDE stores/plugins/mirror', () => {
expect(store.dispatch).toHaveBeenCalledWith(ACTION_UPLOAD);
});
it('does nothing when ide.files.change is emitted with "update"', () => {
eventHub.$emit(FILES_CHANGE_EVENT, createTriggerUpdatePayload('foo'));
jest.runAllTimers();
expect(store.dispatch).not.toHaveBeenCalledWith(ACTION_UPLOAD);
});
describe('when session stops', () => {
beforeEach(() => {
store.commit(`terminal/${SET_SESSION_STATUS}`, STOPPING);
......
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