Skip to content

Commit ea7ba6b

Browse files
fix adding operations to non existing models
1 parent 2b8d12d commit ea7ba6b

File tree

8 files changed

+86
-130
lines changed

8 files changed

+86
-130
lines changed

packages/app/src/app/overmind/effects/vscode/ModelsHandler.ts

Lines changed: 43 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,22 @@ export type OnFileChangeData = {
3434
model?: any;
3535
};
3636

37+
export type OnOperationAppliedData = {
38+
moduleShortid: string;
39+
title: string;
40+
code: string;
41+
model: any;
42+
};
43+
3744
export type OnFileChangeCallback = (data: OnFileChangeData) => void;
3845

46+
export type OnOperationAppliedCallback = (data: OnOperationAppliedData) => void;
47+
3948
export class ModelsHandler {
4049
private modelAddedListener: { dispose: Function };
4150
private modelRemovedListener: { dispose: Function };
4251
private onChangeCallback: OnFileChangeCallback;
52+
private onOperationAppliedCallback: OnOperationAppliedCallback;
4353
private sandbox: Sandbox;
4454
private editorApi;
4555
private monaco;
@@ -55,11 +65,18 @@ export class ModelsHandler {
5565
};
5666
} = {};
5767

58-
constructor(editorApi, monaco, sandbox: Sandbox, cb: OnFileChangeCallback) {
68+
constructor(
69+
editorApi,
70+
monaco,
71+
sandbox: Sandbox,
72+
onChangeCallback: OnFileChangeCallback,
73+
onOperationAppliedCallback: OnOperationAppliedCallback
74+
) {
5975
this.editorApi = editorApi;
6076
this.monaco = monaco;
6177
this.sandbox = sandbox;
62-
this.onChangeCallback = cb;
78+
this.onChangeCallback = onChangeCallback;
79+
this.onOperationAppliedCallback = onOperationAppliedCallback;
6380
this.listenForChanges();
6481
}
6582

@@ -77,80 +94,43 @@ export class ModelsHandler {
7794
public changeModule = async (module: Module) => {
7895
if (getCurrentModelPath(this.editorApi) !== module.path) {
7996
const file = await this.editorApi.openFile(module.path);
80-
8197
return file.getModel();
8298
}
8399

84100
return Promise.resolve(getModel(this.editorApi));
85101
};
86102

87-
public applyOperations(operations: { [moduleShortid: string]: any }) {
88-
const operationsJSON = operations.toJSON ? operations.toJSON() : operations;
103+
public async applyOperation(moduleShortid: string, operation: any) {
104+
const module = this.sandbox.modules.find(m => m.shortid === moduleShortid);
89105

90-
return Promise.all(
91-
Object.keys(operationsJSON).map(moduleShortid => {
92-
const operation = TextOperation.fromJSON(operationsJSON[moduleShortid]);
93-
94-
const foundModule = this.sandbox.modules.find(
95-
m => m.shortid === moduleShortid
96-
);
97-
98-
if (!foundModule) {
99-
return Promise.resolve();
100-
}
101-
102-
const modulePath = '/sandbox' + foundModule.path;
103-
104-
const modelEditor = this.editorApi.editorService.editors.find(
105-
editor => editor.resource && editor.resource.path === modulePath
106-
);
106+
if (!module) {
107+
return;
108+
}
107109

108-
// Apply the code to the current module code itself
109-
const module = this.sandbox.modules.find(
110-
m => m.shortid === moduleShortid
111-
);
110+
const modulePath = '/sandbox' + module.path;
112111

113-
if (!modelEditor) {
114-
if (!module) {
115-
return Promise.resolve();
116-
}
112+
const modelEditor = this.editorApi.editorService.editors.find(
113+
editor => editor.resource && editor.resource.path === modulePath
114+
);
117115

118-
try {
119-
const code = operation.apply(module.code || '');
120-
121-
// Should this run? We are applying changes from the outside,
122-
// should not trigger a change?
123-
this.onChangeCallback({
124-
code,
125-
moduleShortid: module.shortid,
126-
title: module.title,
127-
});
128-
} catch (e) {
129-
throw new Error('Module state mismatch');
130-
}
116+
let model;
131117

132-
return Promise.resolve();
133-
}
118+
if (modelEditor) {
119+
model = (await modelEditor.textModelReference).object;
120+
} else {
121+
model = await this.editorApi.textFileService.models.loadOrCreate(
122+
this.monaco.Uri.file(modulePath)
123+
);
124+
}
134125

135-
return modelEditor.textModelReference.then(model => {
136-
this.applyOperationToModel(
137-
operation,
138-
false,
139-
model.object.textEditorModel
140-
);
126+
this.applyOperationToModel(operation, false, model.textEditorModel);
141127

142-
if (module) {
143-
// Should this really run? We have applied from the outside
144-
this.onChangeCallback({
145-
code: model.object.textEditorModel.getValue(),
146-
moduleShortid: module.shortid,
147-
title: module.title,
148-
model,
149-
});
150-
}
151-
});
152-
})
153-
);
128+
this.onOperationAppliedCallback({
129+
code: model.textEditorModel.getValue(),
130+
moduleShortid: module.shortid,
131+
title: module.title,
132+
model: model.textEditorModel,
133+
});
154134
}
155135

156136
public updateUserSelections(

packages/app/src/app/overmind/effects/vscode/index.ts

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,25 +18,30 @@ import { listen } from 'codesandbox-api';
1818
import FontFaceObserver from 'fontfaceobserver';
1919
import * as childProcess from 'node-services/lib/child_process';
2020

21+
import { VIM_EXTENSION_ID } from './constants';
2122
import {
2223
initializeCustomTheme,
2324
initializeExtensionsFolder,
2425
initializeSettings,
2526
initializeThemeCache,
2627
} from './initializers';
2728
import { Linter } from './Linter';
28-
import { ModelsHandler, OnFileChangeData } from './ModelsHandler';
29+
import {
30+
ModelsHandler,
31+
OnFileChangeData,
32+
OnOperationAppliedData,
33+
} from './ModelsHandler';
2934
import sandboxFsSync from './sandboxFsSync';
3035
import { getSelection } from './utils';
3136
import loadScript from './vscode-script-loader';
3237
import { Workbench } from './Workbench';
33-
import { VIM_EXTENSION_ID } from './constants';
3438

3539
export type VsCodeOptions = {
3640
getCurrentSandbox: () => Sandbox;
3741
getCurrentModule: () => Module;
3842
getSandboxFs: () => SandboxFs;
3943
onCodeChange: (data: OnFileChangeData) => void;
44+
onOperationApplied: (data: OnOperationAppliedData) => void;
4045
onSelectionChange: (selection: any) => void;
4146
reaction: Reaction;
4247
// These two should be removed
@@ -193,20 +198,19 @@ export class VSCodeEffect {
193198
}
194199
}
195200

196-
public async applyOperations(operations: {
197-
[x: string]: (string | number)[];
198-
}) {
201+
public async applyOperation(
202+
moduleShortid: string,
203+
operation: (string | number)[]
204+
) {
199205
this.isApplyingCode = true;
200206

201-
// The call to "apployOperations" should "try" and treat error as "moduleStateMismatch"
202-
// this.props.onModuleStateMismatch();
203-
204207
try {
205-
await this.modelsHandler.applyOperations(operations);
208+
await this.modelsHandler.applyOperation(moduleShortid, operation);
206209
} catch (error) {
207210
this.isApplyingCode = false;
208211
throw error;
209212
}
213+
this.isApplyingCode = false;
210214
}
211215

212216
public updateOptions(options: { readOnly: boolean }) {
@@ -245,7 +249,8 @@ export class VSCodeEffect {
245249
this.editorApi,
246250
this.monaco,
247251
sandbox,
248-
this.onFileChange
252+
this.onFileChange,
253+
this.onOperationApplied
249254
);
250255

251256
this.fs.sync();
@@ -606,13 +611,16 @@ export class VSCodeEffect {
606611
}
607612
};
608613

609-
private onFileChange = (data: OnFileChangeData) => {
610-
if (this.isApplyingCode) {
611-
return;
614+
private onOperationApplied = (data: OnOperationAppliedData) => {
615+
if (data.moduleShortid === this.options.getCurrentModule().shortid) {
616+
this.lint(data.title, data.model);
612617
}
613618

614-
this.lint(data.title, data.model);
619+
this.options.onOperationApplied(data);
620+
};
615621

622+
private onFileChange = (data: OnFileChangeData) => {
623+
this.lint(data.title, data.model);
616624
this.options.onCodeChange(data);
617625
};
618626

packages/app/src/app/overmind/namespaces/editor/actions.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,24 @@ export const codeSaved: AsyncAction<{
171171
}
172172
);
173173

174+
export const onOperationApplied: Action<{
175+
moduleShortid: string;
176+
code: string;
177+
}> = ({ state, actions }, { code, moduleShortid }) => {
178+
const module = state.editor.currentSandbox.modules.find(
179+
m => m.shortid === moduleShortid
180+
);
181+
182+
if (!module) {
183+
return;
184+
}
185+
186+
actions.editor.internal.setModuleCode({
187+
module,
188+
code,
189+
});
190+
};
191+
174192
export const codeChanged: Action<{
175193
moduleShortid: string;
176194
code: string;
@@ -332,11 +350,6 @@ export const moduleSelected: Action<{
332350
actions.editor.internal.setCurrentModule(module);
333351

334352
if (state.live.isLive) {
335-
/*
336-
state.editor.pendingUserSelections = actions.live.internal.getSelectionsForModule(
337-
module
338-
);
339-
*/
340353
effects.vscode.updateUserSelections(
341354
actions.live.internal.getSelectionsForModule(module)
342355
);

packages/app/src/app/overmind/namespaces/editor/state.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { ViewConfig } from '@codesandbox/common/lib/templates/template';
55
import {
66
DevToolsTabPosition,
77
DiffTab,
8-
EditorSelection,
98
Module,
109
ModuleCorrection,
1110
ModuleError,
@@ -38,10 +37,6 @@ type State = {
3837
error: string | null;
3938
isResizing: boolean;
4039
changedModuleShortids: string[];
41-
pendingOperations: {
42-
[id: string]: Array<string | number>;
43-
};
44-
pendingUserSelections: EditorSelection[];
4540
currentTabId: string;
4641
tabs: Tabs;
4742
errors: ModuleError[];
@@ -88,8 +83,6 @@ export const state: State = {
8883
errors: [],
8984
sessionFrozen: true,
9085
corrections: [],
91-
pendingOperations: {},
92-
pendingUserSelections: [],
9386
isInProjectView: false,
9487
forceRender: 0,
9588
initialPath: '/',

packages/app/src/app/overmind/namespaces/live/actions.ts

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { LiveMessage, LiveMessageEvent } from '@codesandbox/common/lib/types';
22
import { Action, AsyncAction, Operator } from 'app/overmind';
33
import { withLoadApp } from 'app/overmind/factories';
4-
import { TextOperation } from 'ot';
54
import { filter, fork, pipe } from 'overmind';
65

76
import eventToTransform from '../../utils/event-to-transform';
@@ -101,25 +100,9 @@ export const applyTransformation: Action<{
101100
operation: any;
102101
moduleShortid: string;
103102
}> = ({ state, effects }, { operation, moduleShortid }) => {
104-
let pendingOperation;
105-
106-
const existingPendingOperation =
107-
state.editor.pendingOperations[moduleShortid];
108-
109-
if (existingPendingOperation) {
110-
pendingOperation = TextOperation.fromJSON(existingPendingOperation)
111-
.compose(operation)
112-
.toJSON();
113-
} else {
114-
pendingOperation = operation.toJSON();
115-
}
116-
117-
state.editor.pendingOperations[moduleShortid] = pendingOperation;
118-
effects.vscode.applyOperations(state.editor.pendingOperations);
119-
// this.props.signals.live.onOperationApplied();
120-
// Removed this action... but this whole object thing here does not seem to make sense
121-
state.editor.pendingOperations = {};
122103
state.live.receivingCode = false;
104+
105+
effects.vscode.applyOperation(moduleShortid, operation);
123106
};
124107

125108
export const onCodeReceived: Action = ({ state }) => {
@@ -151,17 +134,6 @@ export const onSelectionChanged: Action<any> = (
151134
}
152135
};
153136

154-
/*
155-
Not run anymore
156-
export const onSelectionDecorationsApplied: Action = ({ state }) => {
157-
// We only clear it out if we actually need to. There is a reaction
158-
// running that reacts to any change here
159-
if (state.editor.pendingUserSelections.length) {
160-
state.editor.pendingUserSelections = [];
161-
}
162-
};
163-
*/
164-
165137
export const onModeChanged: Action<{ mode: string }> = (
166138
{ state, effects },
167139
{ mode }

packages/app/src/app/overmind/namespaces/live/internalActions.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,6 @@ export const clearUserSelections: Action<any> = (
2424
color: null,
2525
},
2626
]);
27-
/*
28-
state.editor.pendingUserSelections.push({
29-
userId,
30-
selection: null,
31-
name: null,
32-
color: null,
33-
});
34-
*/
3527
}
3628
}
3729
};

packages/app/src/app/overmind/namespaces/live/liveMessageOperators.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -343,9 +343,6 @@ export const onLiveMode: Operator<
343343
state.live.roomInfo.mode = data.mode;
344344
}
345345
actions.live.internal.clearUserSelections(null);
346-
state.editor.pendingUserSelections = state.editor.pendingUserSelections.concat(
347-
actions.live.internal.getSelectionsForModule(state.editor.currentModule)
348-
);
349346
});
350347

351348
export const onLiveChatEnabled: Operator<

packages/app/src/app/overmind/onInitialize.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export const onInitialize: OnInitialize = async (
6464
getCurrentSandbox: () => state.editor.currentSandbox,
6565
getCurrentModule: () => state.editor.currentModule,
6666
getSandboxFs: () => state.editor.modulesByPath,
67+
onOperationApplied: actions.editor.onOperationApplied,
6768
onCodeChange: actions.editor.codeChanged,
6869
onSelectionChange: actions.live.onSelectionChanged,
6970
reaction: overmindInstance.reaction,

0 commit comments

Comments
 (0)