Skip to content

Commit 4685573

Browse files
DonJayamanneDonJayamanne
authored
Complete refactoring of creation and queueing of actions (#10036)
* Some how I had not included some of the other remaining arg.queueAction into the original PR #10019 * This PR merely brings them from (I had these in the same branch, but failed to include them). * Same as #10019, just a refactor to use a common function to create an action and dispatch it in one step. & removed some of the old syncing stuff.
1 parent 70e2431 commit 4685573

File tree

12 files changed

+122
-81
lines changed

12 files changed

+122
-81
lines changed

‎src/client/datascience/interactive-common/interactiveBase.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import { CancellationToken, ConfigurationTarget, Event, EventEmitter, Memento, P
1111
import { Disposable } from 'vscode-jsonrpc';
1212

1313
import { ServerStatus } from '../../../datascience-ui/interactive-common/mainState';
14-
import { CommonActionType } from '../../../datascience-ui/interactive-common/redux/reducers/types';
1514
import { IApplicationShell, ICommandManager, IDocumentManager, ILiveShareApi, IWebPanelProvider, IWorkspaceService } from '../../common/application/types';
1615
import { CancellationError } from '../../common/cancellation';
1716
import { EXTENSION_ROOT_DIR, PYTHON_LANGUAGE } from '../../common/constants';
@@ -44,7 +43,7 @@ import { JupyterInstallError } from '../jupyter/jupyterInstallError';
4443
import { JupyterSelfCertsError } from '../jupyter/jupyterSelfCertsError';
4544
import { JupyterKernelPromiseFailedError } from '../jupyter/kernels/jupyterKernelPromiseFailedError';
4645
import { LiveKernelModel } from '../jupyter/kernels/types';
47-
import { CssMessages, SharedMessages } from '../messages';
46+
import { CssMessages } from '../messages';
4847
import { ProgressReporter } from '../progress/progressReporter';
4948
import {
5049
CellState,
@@ -74,7 +73,6 @@ import {
7473
} from '../types';
7574
import { WebViewHost } from '../webViewHost';
7675
import { InteractiveWindowMessageListener } from './interactiveWindowMessageListener';
77-
import { BaseReduxActionPayload } from './types';
7876

7977
@injectable()
8078
export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapping> implements IInteractiveBase {
@@ -178,11 +176,6 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
178176
// tslint:disable-next-line: no-any no-empty cyclomatic-complexity max-func-body-length
179177
public onMessage(message: string, payload: any) {
180178
switch (message) {
181-
case InteractiveWindowMessages.Sync:
182-
// tslint:disable-next-line: no-any
183-
const syncPayload = payload as { type: InteractiveWindowMessages | SharedMessages | CommonActionType; payload: BaseReduxActionPayload<any> };
184-
this.postMessageInternal(syncPayload.type, syncPayload.payload).ignoreErrors();
185-
break;
186179
case InteractiveWindowMessages.GotoCodeCell:
187180
this.handleMessage(message, payload, this.gotoCode);
188181
break;

‎src/client/datascience/interactive-common/interactiveWindowMessageListener.ts

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import { InteractiveWindowMessages, InteractiveWindowRemoteMessages } from './in
1515

1616
// This class listens to messages that come from the local Python Interactive window
1717
export class InteractiveWindowMessageListener implements IWebPanelMessageListener {
18-
private static handlers = new Map<InteractiveWindowMessageListener, (message: string, payload: any) => void>();
1918
private postOffice: PostOffice;
2019
private disposedCallback: () => void;
2120
private callback: (message: string, payload: any) => void;
@@ -41,7 +40,6 @@ export class InteractiveWindowMessageListener implements IWebPanelMessageListene
4140
this.interactiveWindowMessages.forEach(m => {
4241
this.postOffice.registerCallback(m, a => callback(m, a)).ignoreErrors();
4342
});
44-
InteractiveWindowMessageListener.handlers.set(this, callback);
4543
}
4644

4745
public async dispose() {
@@ -50,20 +48,6 @@ export class InteractiveWindowMessageListener implements IWebPanelMessageListene
5048
}
5149

5250
public onMessage(message: string, payload: any) {
53-
if (message === InteractiveWindowMessages.Sync) {
54-
// const syncPayload = payload as BaseReduxActionPayload;
55-
Array.from(InteractiveWindowMessageListener.handlers.keys()).forEach(item => {
56-
if (item === this) {
57-
return;
58-
}
59-
// Temporarily disabled.
60-
// const cb = InteractiveWindowMessageListener.handlers.get(item);
61-
// if (cb) {
62-
// cb(InteractiveWindowMessages.Sync, { type: message, payload: syncPayload });
63-
// }
64-
});
65-
return;
66-
}
6751
// We received a message from the local webview. Broadcast it to everybody if it's a remote message
6852
if (InteractiveWindowRemoteMessages.indexOf(message) >= 0) {
6953
this.postOffice.postCommand(message, payload).ignoreErrors();

‎src/client/datascience/interactive-common/synchronization.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { CommonActionType, CommonActionTypeMapping } from '../../../datascience-ui/interactive-common/redux/reducers/types';
22
import { CssMessages, SharedMessages } from '../messages';
33
import { IInteractiveWindowMapping, InteractiveWindowMessages } from './interactiveWindowTypes';
4-
import { BaseReduxActionPayload } from './types';
54

65
// Copyright (c) Microsoft Corporation. All rights reserved.
76
// Licensed under the MIT License.
@@ -172,11 +171,30 @@ const messageWithMessageTypes: MessageMapping<IInteractiveWindowMapping> & Messa
172171
[SharedMessages.UpdateSettings]: MessageType.userAction
173172
};
174173

175-
export function isActionPerformedByUser(action: BaseReduxActionPayload<{}> | BaseReduxActionPayload<never>) {
176-
return action.messageType === undefined;
174+
/**
175+
* If the original message was a sync message, then do not send messages to extension.
176+
* We allow messages to be sent to extension ONLY when the original message was triggered by the user.
177+
*
178+
* @export
179+
* @param {MessageType} [messageType]
180+
* @returns
181+
*/
182+
export function checkToPostBasedOnOriginalMessageType(messageType?: MessageType): boolean {
183+
if (!messageType) {
184+
return true;
185+
}
186+
if (
187+
(messageType & MessageType.syncAcrossSameNotebooks) === MessageType.syncAcrossSameNotebooks ||
188+
(messageType & MessageType.syncWithLiveShare) === MessageType.syncWithLiveShare
189+
) {
190+
return false;
191+
}
192+
193+
return true;
177194
}
178195

179196
export function shouldRebroadcast(message: keyof IInteractiveWindowMapping): [boolean, MessageType] {
197+
// Get the configured type for this message (whether it should be re-broadcasted or not).
180198
const messageType: MessageType | undefined = messageWithMessageTypes[message];
181199
// Support for liveshare is turned off for now, we can enable that later.
182200
// I.e. we only support synchronizing across editors in the same session.

‎src/datascience-ui/history-react/redux/actions.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22
// Licensed under the MIT License.
33
'use strict';
44

5-
import { InteractiveWindowMessages } from '../../../client/datascience/interactive-common/interactiveWindowTypes';
5+
import { IInteractiveWindowMapping, InteractiveWindowMessages } from '../../../client/datascience/interactive-common/interactiveWindowTypes';
66
import { IJupyterVariable, IJupyterVariablesRequest } from '../../../client/datascience/types';
7-
import { createIncomingAction, createIncomingActionWithPayload } from '../../interactive-common/redux/helpers';
87
import {
98
CommonAction,
109
CommonActionType,
10+
CommonActionTypeMapping,
1111
ICellAction,
1212
ICodeAction,
1313
ICodeCreatedAction,
@@ -18,6 +18,16 @@ import {
1818
} from '../../interactive-common/redux/reducers/types';
1919
import { IMonacoModelContentChangeEvent } from '../../react-common/monacoHelpers';
2020

21+
// This function isn't made common and not exported, to ensure it isn't used elsewhere.
22+
function createIncomingActionWithPayload<M extends IInteractiveWindowMapping & CommonActionTypeMapping, K extends keyof M>(type: K, data: M[K]): CommonAction<M[K]> {
23+
// tslint:disable-next-line: no-any
24+
return { type, payload: { data, messageDirection: 'incoming' } as any } as any;
25+
}
26+
// This function isn't made common and not exported, to ensure it isn't used elsewhere.
27+
function createIncomingAction(type: CommonActionType | InteractiveWindowMessages): CommonAction {
28+
return { type, payload: { messageDirection: 'incoming', data: undefined } };
29+
}
30+
2131
// See https://react-redux.js.org/using-react-redux/connect-mapdispatch#defining-mapdispatchtoprops-as-an-object
2232
export const actionCreators = {
2333
restartKernel: (): CommonAction => createIncomingAction(CommonActionType.RESTART_KERNEL),

‎src/datascience-ui/interactive-common/redux/helpers.ts

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@
55

66
import * as Redux from 'redux';
77
import { IInteractiveWindowMapping, InteractiveWindowMessages } from '../../../client/datascience/interactive-common/interactiveWindowTypes';
8+
import { checkToPostBasedOnOriginalMessageType, MessageType, shouldRebroadcast } from '../../../client/datascience/interactive-common/synchronization';
89
import { BaseReduxActionPayload } from '../../../client/datascience/interactive-common/types';
910
import { CssMessages, SharedMessages } from '../../../client/datascience/messages';
1011
import { QueueAnotherFunc } from '../../react-common/reduxUtils';
11-
import { CommonAction, CommonActionType, CommonActionTypeMapping } from './reducers/types';
12+
import { CommonActionType, CommonActionTypeMapping } from './reducers/types';
1213

1314
const AllowedMessages = [...Object.values(InteractiveWindowMessages), ...Object.values(CssMessages), ...Object.values(SharedMessages), ...Object.values(CommonActionType)];
1415
export function isAllowedMessage(message: string) {
@@ -19,20 +20,32 @@ export function isAllowedAction(action: Redux.AnyAction) {
1920
return isAllowedMessage(action.type);
2021
}
2122

22-
export function createIncomingActionWithPayload<M extends IInteractiveWindowMapping & CommonActionTypeMapping, K extends keyof M>(type: K, data: M[K]): CommonAction<M[K]> {
23-
// tslint:disable-next-line: no-any
24-
return { type, payload: { data, messageDirection: 'incoming' } as any } as any;
25-
}
26-
export function createIncomingAction(type: CommonActionType | InteractiveWindowMessages): CommonAction {
27-
return { type, payload: { messageDirection: 'incoming', data: undefined } };
28-
}
29-
3023
type ReducerArg = {
3124
// tslint:disable-next-line: no-any
3225
queueAction: QueueAnotherFunc<any>;
3326
// tslint:disable-next-line: no-any
3427
payload?: BaseReduxActionPayload<any>;
3528
};
29+
30+
export function queueIncomingActionWithPayload<M extends IInteractiveWindowMapping & CommonActionTypeMapping, K extends keyof M>(
31+
originalReducerArg: ReducerArg,
32+
type: K,
33+
data: M[K]
34+
): void {
35+
if (!checkToPostBasedOnOriginalMessageType(originalReducerArg.payload?.messageType)) {
36+
return;
37+
}
38+
39+
// tslint:disable-next-line: no-any
40+
const action = { type, payload: { data, messageDirection: 'incoming' } as any } as any;
41+
originalReducerArg.queueAction(action);
42+
}
43+
44+
export function queueIncomingAction<M extends IInteractiveWindowMapping & CommonActionTypeMapping, K extends keyof M>(originalReducerArg: ReducerArg, type: K): void {
45+
// tslint:disable-next-line: no-any
46+
queueIncomingActionWithPayload(originalReducerArg, type as any, undefined);
47+
}
48+
3649
/**
3750
* Post a message to the extension (via dispatcher actions).
3851
*/
@@ -44,10 +57,15 @@ export function postActionToExtension<K, M extends IInteractiveWindowMapping, T
4457
export function postActionToExtension<K, M extends IInteractiveWindowMapping, T extends keyof M = keyof M>(originalReducerArg: ReducerArg, message: T, payload?: M[T]): void;
4558
// tslint:disable-next-line: no-any
4659
export function postActionToExtension(originalReducerArg: ReducerArg, message: any, payload?: any) {
60+
if (!checkToPostBasedOnOriginalMessageType(originalReducerArg.payload?.messageType)) {
61+
return;
62+
}
63+
4764
// tslint:disable-next-line: no-any
4865
const newPayload: BaseReduxActionPayload<any> = ({
4966
data: payload,
50-
messageDirection: 'outgoing'
67+
messageDirection: 'outgoing',
68+
messageType: MessageType.userAction
5169
// tslint:disable-next-line: no-any
5270
} as any) as BaseReduxActionPayload<any>;
5371
const action = { type: CommonActionType.PostOutgoingMessage, payload: { payload: newPayload, type: message } };
@@ -61,27 +79,28 @@ export function unwrapPostableAction(action: Redux.AnyAction): { type: keyof IIn
6179
}
6280

6381
export function reBroadcastMessageIfRequired(
64-
_dispatcher: Function,
82+
dispatcher: Function,
6583
message: InteractiveWindowMessages | SharedMessages | CommonActionType | CssMessages,
6684
payload?: BaseReduxActionPayload<{}>
6785
) {
68-
if (typeof payload?.messageType === 'number' || message === InteractiveWindowMessages.Sync) {
86+
if (
87+
message === InteractiveWindowMessages.Sync ||
88+
payload?.messageType === MessageType.syncAcrossSameNotebooks ||
89+
payload?.messageType === MessageType.syncWithLiveShare ||
90+
payload?.messageDirection === 'outgoing'
91+
) {
6992
return;
7093
}
71-
if (payload?.messageDirection === 'outgoing') {
72-
return;
94+
// Check if we need to re-broadcast this message to other editors/sessions.
95+
// tslint:disable-next-line: no-any
96+
const result = shouldRebroadcast(message as any);
97+
if (result[0]) {
98+
// Mark message as incoming, to indicate this will be sent into the other webviews.
99+
// tslint:disable-next-line: no-any
100+
const syncPayloadData: BaseReduxActionPayload<any> = { data: payload?.data, messageType: result[1], messageDirection: 'incoming' };
101+
// tslint:disable-next-line: no-any
102+
const syncPayload = { type: message, payload: syncPayloadData } as any;
103+
// Send this out.
104+
dispatcher(InteractiveWindowMessages.Sync, syncPayload);
73105
}
74-
// Temporarily disabled.
75-
// // Check if we need to re-broadcast this message to other editors/sessions.
76-
// // tslint:disable-next-line: no-any
77-
// const result = shouldRebroadcast(message as any);
78-
// if (result[0]) {
79-
// // Mark message as incoming, to indicate this will be sent into the other webviews.
80-
// // tslint:disable-next-line: no-any
81-
// const syncPayloadData: BaseReduxActionPayload<any> = { data: payload?.data, messageType: result[1], messageDirection: 'incoming' };
82-
// // tslint:disable-next-line: no-any
83-
// const syncPayload = { type: message, payload: syncPayloadData } as any;
84-
// // Send this out.
85-
// dispatcher(InteractiveWindowMessages.Sync, syncPayload);
86-
// }
87106
}

‎src/datascience-ui/interactive-common/redux/postOffice.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export function generatePostOfficeSendReducer(postOffice: PostOffice): Redux.Red
2323
const payload: BaseReduxActionPayload<{}> | undefined = action.payload;
2424
// Do not rebroadcast messages that have been sent through as part of a synchronization packet.
2525
// If `messageType` is a number, then its some part of a synchronization packet.
26-
if (payload?.messageDirection === 'incoming' && typeof payload?.messageType !== 'number') {
26+
if (payload?.messageDirection === 'incoming') {
2727
// We can delay this, first focus on UX perf.
2828
setTimeout(() => {
2929
reBroadcastMessageIfRequired(postOffice.sendMessage.bind(postOffice), action.type, action?.payload);

‎src/datascience-ui/interactive-common/redux/reducers/monaco.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { PostOffice } from '../../../react-common/postOffice';
2020
import { combineReducers, QueuableAction, ReducerArg, ReducerFunc } from '../../../react-common/reduxUtils';
2121
import { IntellisenseProvider } from '../../intellisenseProvider';
2222
import { initializeTokenizer, registerMonacoLanguage } from '../../tokenizer';
23-
import { createIncomingAction } from '../helpers';
23+
import { queueIncomingAction } from '../helpers';
2424
import { CommonActionType, ICodeCreatedAction, IEditCellAction } from './types';
2525

2626
export interface IMonacoState {
@@ -63,7 +63,7 @@ function finishTokenizer<T>(buffer: ArrayBuffer, tmJson: string, arg: MonacoRedu
6363
if (e) {
6464
logMessage(`ERROR from onigasm: ${e}`);
6565
}
66-
arg.queueAction(createIncomingAction(InteractiveWindowMessages.MonacoReady));
66+
queueIncomingAction(arg, InteractiveWindowMessages.MonacoReady);
6767
}).ignoreErrors();
6868
}
6969

‎src/datascience-ui/interactive-common/redux/store.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as Redux from 'redux';
66
import { createLogger } from 'redux-logger';
77
import { Identifiers } from '../../../client/datascience/constants';
88
import { InteractiveWindowMessages } from '../../../client/datascience/interactive-common/interactiveWindowTypes';
9+
import { MessageType } from '../../../client/datascience/interactive-common/synchronization';
910
import { BaseReduxActionPayload } from '../../../client/datascience/interactive-common/types';
1011
import { CssMessages } from '../../../client/datascience/messages';
1112
import { CellState } from '../../../client/datascience/types';
@@ -75,7 +76,11 @@ function createSendInfoMiddleware(): Redux.Middleware<{}, IStore> {
7576
const afterState = store.getState();
7677

7778
// If the action is part of a sync message, then do not send it to the extension.
78-
if (action.payload && typeof (action.payload as BaseReduxActionPayload).messageType === 'number') {
79+
const messageType = (action?.payload as BaseReduxActionPayload).messageType ?? MessageType.userAction;
80+
const isSyncMessage =
81+
(messageType & MessageType.syncAcrossSameNotebooks) === MessageType.syncAcrossSameNotebooks &&
82+
(messageType & MessageType.syncAcrossSameNotebooks) === MessageType.syncWithLiveShare;
83+
if (isSyncMessage) {
7984
return res;
8085
}
8186

@@ -296,10 +301,14 @@ export function createStore<M>(skipDefault: boolean, baseTheme: string, testMode
296301
if (isAllowedMessage(message)) {
297302
const basePayload: BaseReduxActionPayload = { data: payload };
298303
if (message === InteractiveWindowMessages.Sync) {
304+
// This is a message that has been sent from extension purely for synchronization purposes.
299305
// Unwrap the message.
300306
message = payload.type;
301-
basePayload.messageType = payload.payload.messageType;
307+
basePayload.messageType = payload.payload.messageType ?? MessageType.syncAcrossSameNotebooks;
302308
basePayload.data = payload.payload.data;
309+
} else {
310+
// Messages result of some user action.
311+
basePayload.messageType = basePayload.messageType ?? MessageType.userAction;
303312
}
304313
store.dispatch({ type: message, payload: basePayload });
305314
}

‎src/datascience-ui/native-editor/redux/actions.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
// Licensed under the MIT License.
33
'use strict';
44
import * as uuid from 'uuid/v4';
5-
import { InteractiveWindowMessages, NativeCommandType } from '../../../client/datascience/interactive-common/interactiveWindowTypes';
5+
import { IInteractiveWindowMapping, InteractiveWindowMessages, NativeCommandType } from '../../../client/datascience/interactive-common/interactiveWindowTypes';
66
import { IJupyterVariable, IJupyterVariablesRequest } from '../../../client/datascience/types';
77
import { CursorPos } from '../../interactive-common/mainState';
8-
import { createIncomingAction, createIncomingActionWithPayload } from '../../interactive-common/redux/helpers';
98
import {
109
CommonAction,
1110
CommonActionType,
11+
CommonActionTypeMapping,
1212
ICellAction,
1313
ICellAndCursorAction,
1414
ICodeAction,
@@ -20,6 +20,16 @@ import {
2020
} from '../../interactive-common/redux/reducers/types';
2121
import { IMonacoModelContentChangeEvent } from '../../react-common/monacoHelpers';
2222

23+
// This function isn't made common and not exported, to ensure it isn't used elsewhere.
24+
function createIncomingActionWithPayload<M extends IInteractiveWindowMapping & CommonActionTypeMapping, K extends keyof M>(type: K, data: M[K]): CommonAction<M[K]> {
25+
// tslint:disable-next-line: no-any
26+
return { type, payload: { data, messageDirection: 'incoming' } as any } as any;
27+
}
28+
// This function isn't made common and not exported, to ensure it isn't used elsewhere.
29+
function createIncomingAction(type: CommonActionType | InteractiveWindowMessages): CommonAction {
30+
return { type, payload: { messageDirection: 'incoming', data: undefined } };
31+
}
32+
2333
// See https://react-redux.js.org/using-react-redux/connect-mapdispatch#defining-mapdispatchtoprops-as-an-object
2434
export const actionCreators = {
2535
addCell: () => createIncomingActionWithPayload(CommonActionType.ADD_AND_FOCUS_NEW_CELL, { newCellId: uuid() }),

0 commit comments

Comments
 (0)