Skip to content

Commit

Permalink
Remove console log usage (#10341)
Browse files Browse the repository at this point in the history
* Removing some from the jupyter extension

* More removals of console.log

* Add news entry

* Update telemetry

* Update telemetry again

* Update to latest ipywidgets
  • Loading branch information
rchiodo authored Jun 7, 2022
1 parent a885a50 commit 0b3ede8
Show file tree
Hide file tree
Showing 11 changed files with 21 additions and 42 deletions.
2 changes: 1 addition & 1 deletion TELEMETRY.md
Original file line number Diff line number Diff line change
Expand Up @@ -6772,7 +6772,7 @@ No properties for event

[src/webviews/extension-side/variablesView/variableView.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/variablesView/variableView.ts)
```typescript
console.log(`Done initializing variables`);
this.dataViewerChecker = new DataViewerChecker(configuration, appShell);
}

@captureTelemetry(Telemetry.NativeVariableViewLoaded)
Expand Down
1 change: 1 addition & 0 deletions news/3 Code Health/10202.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove usage of console.log in renderers.
21 changes: 10 additions & 11 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2159,7 +2159,7 @@
"@lumino/widgets": "^1.28.0",
"@nteract/messaging": "^7.0.0",
"@vscode/extension-telemetry": "^0.5.2",
"@vscode/jupyter-ipywidgets": "1.0.8",
"@vscode/jupyter-ipywidgets": "^1.0.9",
"@vscode/jupyter-lsp-middleware": "0.2.38",
"ajv-keywords": "^3.5.2",
"ansi-to-html": "^0.6.7",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { KernelMessage } from '@jupyterlab/services';
import * as uuid from 'uuid/v4';
import { Event, EventEmitter, NotebookDocument } from 'vscode';
import type { Data as WebSocketData } from 'ws';
import { traceInfoIfCI, traceVerbose, traceError } from '../../platform/logging';
import { traceVerbose, traceError, traceInfo } from '../../platform/logging';
import { IDisposable } from '../../platform/common/types';
import { Deferred, createDeferred } from '../../platform/common/utils/async';
import { noop } from '../../platform/common/utils/misc';
Expand Down Expand Up @@ -106,7 +106,7 @@ export class IPyWidgetMessageDispatcher implements IIPyWidgetMessageDispatcher {
public receiveMessage(message: IPyWidgetMessage): void {
switch (message.message) {
case IPyWidgetMessages.IPyWidgets_logMessage:
traceInfoIfCI(`Widget Message: ${message.payload}`);
traceInfo(`Widget Message: ${message.payload}`);
break;
case IPyWidgetMessages.IPyWidgets_Ready:
this.sendKernelOptions();
Expand Down
1 change: 0 additions & 1 deletion src/webviews/extension-side/variablesView/variableView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ export class VariableView extends WebviewViewHost<IVariableViewPanelMapping> imp
this.documentManager.onDidChangeActiveTextEditor(this.activeTextEditorChanged, this, this.disposables);

this.dataViewerChecker = new DataViewerChecker(configuration, appShell);
console.log(`Done initializing variables`);
}

@captureTelemetry(Telemetry.NativeVariableViewLoaded)
Expand Down
6 changes: 0 additions & 6 deletions src/webviews/webview-side/ipywidgets/common/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { KernelSocketOptions } from '../../../../kernels/types';
import { Deferred, createDeferred } from '../../../../platform/common/utils/async';
import { serializeDataViews, deserializeDataViews } from '../../../../platform/common/utils/serializers';
import { IInteractiveWindowMapping, IPyWidgetMessages } from '../../../../platform/messageTypes';
import { logMessageOnlyOnCI } from '../../react-common/logger';
import { IMessageHandler, PostOffice } from '../../react-common/postOffice';

/* eslint-disable @typescript-eslint/no-explicit-any */
Expand Down Expand Up @@ -324,7 +323,6 @@ class ProxyKernel implements IMessageHandler, Kernel.IKernelConnection {
this.messageHooks.set(msgId, hook);

// Wrap the hook and send it to the real kernel
logMessageOnlyOnCI(`Registering hook for ${msgId}`);
this.realKernel.registerMessageHook(msgId, this.messageHook);
}

Expand All @@ -350,7 +348,6 @@ class ProxyKernel implements IMessageHandler, Kernel.IKernelConnection {
this.lastHookedMessageId = undefined;

// Remove from the real kernel
logMessageOnlyOnCI(`Removing hook for ${msgId}`);
this.realKernel.removeMessageHook(msgId, this.messageHook);
}

Expand Down Expand Up @@ -393,9 +390,6 @@ class ProxyKernel implements IMessageHandler, Kernel.IKernelConnection {
}
private messageHookInterceptor(msg: KernelMessage.IIOPubMessage): boolean | PromiseLike<boolean> {
try {
logMessageOnlyOnCI(
`Message hook callback for ${(msg as any).header.msg_type} and ${(msg.parent_header as any).msg_id}`
);
// Save the active message that is currently being hooked. The Extension
// side needs this information during removeMessageHook so it can delay removal until after a message is called
this.lastHookedMessageId = msg.header.msg_id;
Expand Down
9 changes: 5 additions & 4 deletions src/webviews/webview-side/ipywidgets/kernel/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
IInteractiveWindowMapping,
InteractiveWindowMessages
} from '../../../../platform/messageTypes';
import { logMessage } from '../../react-common/logger';

class WidgetManagerComponent {
private readonly widgetManager: WidgetManager;
Expand Down Expand Up @@ -126,9 +127,9 @@ function renderIPyWidget(
container: HTMLElement,
logger: (message: string) => void
) {
console.log(`Rendering IPyWidget ${outputId} with model ${model.model_id}`);
logger(`Rendering IPyWidget ${outputId} with model ${model.model_id}`);
if (renderedWidgets.has(outputId)) {
return console.error('already rendering');
return logger('already rendering');
}
const output = document.createElement('div');
output.className = 'cell-output cell-output';
Expand Down Expand Up @@ -241,10 +242,10 @@ let capturedContext: KernelMessagingApi | undefined;
// To ensure we initialize after the other scripts, wait for them.
function attemptInitialize(context?: KernelMessagingApi) {
capturedContext = capturedContext || context;
console.log('Attempt Initialize IpyWidgets kernel.js', context);
logMessage(`Attempt Initialize IpyWidgets kernel.js : ${JSON.stringify(context)}`);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
if ((window as any).vscIPyWidgets) {
console.log('IPyWidget kernel initializing...');
logMessage('IPyWidget kernel initializing...');
initialize(capturedContext);
} else {
setTimeout(attemptInitialize, 100);
Expand Down
2 changes: 0 additions & 2 deletions src/webviews/webview-side/ipywidgets/renderer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ function hookupTestScripts(context: RendererContext<unknown>) {
message: 'Hook not registered'
});
}
console.log(`No Widgetentry point`);
return;
}
if (context.postMessage) {
Expand All @@ -76,7 +75,6 @@ function hookupTestScripts(context: RendererContext<unknown>) {
message: 'Hook registered'
});
}
console.log(`Widgetentry point found`);
anyWindow.widgetEntryPoint.initialize(context);
}
function sendRenderOutputItem(context: RendererContext<unknown>, outputItem: OutputItem, element: HTMLElement) {
Expand Down
9 changes: 0 additions & 9 deletions src/webviews/webview-side/react-common/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,6 @@ export function logMessage(message: string) {
messageLogger(message);
}
}
/**
* Logging in production seems to slow down webview (unnecessarily too chatty)
*/
export function logMessageOnlyOnCI(message: string) {
// TODO: No way to check if CI for webview
if (messageLogger) {
messageLogger(message);
}
}

export function setLogger(logger: (message: string) => void) {
messageLogger = logger;
Expand Down
6 changes: 1 addition & 5 deletions src/webviews/webview-side/react-common/postOffice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Subject } from 'rxjs/Subject';
import { VSCodeEvent } from 'vscode-notebook-renderer/events';
import { WebviewMessage } from '../../../platform/common/application/types';
import { IDisposable } from '../../../platform/common/types';
import { logMessage, logMessageOnlyOnCI } from './logger';
import { logMessage } from './logger';

export interface IVsCodeApi {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -77,7 +77,6 @@ class VsCodeMessageApi implements IMessageApi {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
public sendMessage(type: string, payload?: any) {
if (this.vscodeApi) {
logMessageOnlyOnCI(`UI PostOffice Sent ${type}`);
this.vscodeApi.postMessage({ type: type, payload });
} else if (type === 'IPyWidgets_logMessage') {
logMessage(`Logging message ${type}, ${payload}`);
Expand Down Expand Up @@ -224,9 +223,6 @@ export class PostOffice implements IDisposable {
private async handleMessage(msg: WebviewMessage) {
if (this.handlers) {
if (msg) {
if ('type' in msg && typeof msg.type === 'string') {
logMessageOnlyOnCI(`UI PostOffice Received ${msg.type}`);
}
this.subject.next({ type: msg.type, payload: msg.payload });
this.handlers.forEach((h: IMessageHandler | null) => {
if (h) {
Expand Down

0 comments on commit 0b3ede8

Please sign in to comment.