Skip to content

Commit

Permalink
Move NotebookControllerManager into the DI container for web (#9690)
Browse files Browse the repository at this point in the history
* Preliminary work

* Fix build errors and unit tests

* Turn on noImplicitOverrides and create the kernelProvider for web

* Add cellhashprovider to web container

* Fixup DI problem in node version

* Linter problem

* Fix vscode test failures

* Fix launching jupyter server and smoke for web

* Fix unit tests on windows and add some missing bindings for web
  • Loading branch information
rchiodo authored Apr 15, 2022
1 parent 984c9bd commit 261afd9
Show file tree
Hide file tree
Showing 320 changed files with 5,358 additions and 4,362 deletions.
1 change: 0 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ module.exports = {
'src/test/mocks/vsc/strings.ts',
'src/test/mocks/vsc/charCode.ts',
'src/test/mocks/vsc/htmlContent.ts',
'src/test/mocks/vsc/selection.ts',
'src/test/mocks/vsc/position.ts',
'src/test/mocks/vsc/telemetryReporter.ts',
'src/test/mocks/vsc/range.ts',
Expand Down
25 changes: 8 additions & 17 deletions build/webpack/webpack.extension.web.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,6 @@ const config = {
},
module: {
rules: [
{
// JupyterServices imports node-fetch.
test: /@jupyterlab[\\\/]services[\\\/].*js$/,
use: [
{
loader: path.join(__dirname, 'loaders', 'fixNodeFetch.js')
}
]
},
{
test: /\.ts$/,
use: [
{
loader: path.join(__dirname, 'loaders', 'externalizeDependencies.js')
}
]
},
{
test: /\.ts$/,
exclude: /node_modules/,
Expand Down Expand Up @@ -134,6 +117,14 @@ const config = {
path: path.resolve(constants.ExtensionRootDir, 'out'),
libraryTarget: 'commonjs2',
devtoolModuleFilenameTemplate: '../[resource-path]'
},
watchOptions: {
aggregateTimeout: 200,
poll: 1000,
ignored: /node_modules/
},
stats: {
builtAt: true
}
};
// tslint:disable-next-line:no-default-export
Expand Down
20 changes: 17 additions & 3 deletions package-lock.json

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

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2099,7 +2099,7 @@
"compiled": "deemon npm run compile",
"kill-compiled": "deemon --kill npm run compile",
"compile-webviews-watch": "webpack --config ./build/webpack/webpack.datascience-ui.config.js --watch",
"compile-web-watch": "webpack --config ./build/webpack/webpack.extension.web.config.js --stats-error-details --watch",
"compile-web-watch": "webpack --config ./build/webpack/webpack.extension.web.config.js --stats-error-details --watch --progress",
"compile-web": "webpack --config ./build/webpack/webpack.extension.web.config.js",
"compile-web-test": "cross-env VSC_TEST_BUNDLE=true npm run compile-web",
"compile-web-test-watch": "cross-env VSC_TEST_BUNDLE=true npm run compile-web-watch",
Expand Down Expand Up @@ -2286,6 +2286,7 @@
"@types/temp": "^0.8.32",
"@types/tmp": "0.0.33",
"@types/untildify": "^3.0.0",
"@types/url-parse": "^1.4.8",
"@types/uuid": "^3.4.3",
"@types/vscode-notebook-renderer": "^1.60.0",
"@types/webpack-bundle-analyzer": "^2.13.0",
Expand Down
2 changes: 2 additions & 0 deletions src/extension.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ import { OutputChannelLogger } from './platform/logging/outputChannelLogger';
import { ConsoleLogger } from './platform/logging/consoleLogger';
import { FileLogger } from './platform/logging/fileLogger.node';
import { createWriteStream } from 'fs-extra';
import { initializeGlobals as initializeTelemetryGlobals } from './telemetry/telemetry';

durations.codeLoadingTime = stopWatch.elapsedTime;

Expand Down Expand Up @@ -160,6 +161,7 @@ async function activateUnsafe(

const [serviceManager, serviceContainer] = initializeGlobals(context);
activatedServiceContainer = serviceContainer;
initializeTelemetryGlobals(serviceContainer);
const activationPromise = activateComponents(context, serviceManager, serviceContainer);

//===============================================
Expand Down
12 changes: 12 additions & 0 deletions src/extension.web.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ import { sendErrorTelemetry, sendStartupTelemetry } from './platform/startupTele
import { noop } from './platform/common/utils/misc';
import { JUPYTER_OUTPUT_CHANNEL, PythonExtension } from './webviews/webview-side/common/constants';
import { registerTypes as registerPlatformTypes } from './platform/serviceRegistry.web';
import { registerTypes as registerTelemetryTypes } from './telemetry/serviceRegistry.web';
import { registerTypes as registerKernelTypes } from './kernels/serviceRegistry.web';
import { registerTypes as registerNotebookTypes } from './notebooks/serviceRegistry.web';
import { registerTypes as registerInteractiveTypes } from './interactive-window/serviceRegistry.web';
import { registerTypes as registerIntellisenseTypes } from './intellisense/serviceRegistry.web';
import { IExtensionActivationManager } from './platform/activation/types';
import { isCI, isTestExecution, STANDARD_OUTPUT_CHANNEL } from './platform/common/constants';
import { getJupyterOutputChannel } from './platform/devTools/jupyterOutputChannel';
Expand All @@ -72,6 +77,7 @@ import { ServiceContainer } from './platform/ioc/container';
import { ServiceManager } from './platform/ioc/serviceManager';
import { OutputChannelLogger } from './platform/logging/outputChannelLogger';
import { ConsoleLogger } from './platform/logging/consoleLogger';
import { initializeGlobals as initializeTelemetryGlobals } from './telemetry/telemetry';

durations.codeLoadingTime = stopWatch.elapsedTime;

Expand Down Expand Up @@ -146,6 +152,7 @@ async function activateUnsafe(

const [serviceManager, serviceContainer] = initializeGlobals(context);
activatedServiceContainer = serviceContainer;
initializeTelemetryGlobals(serviceContainer);
const activationPromise = activateComponents(context, serviceManager, serviceContainer);

//===============================================
Expand Down Expand Up @@ -270,6 +277,11 @@ async function activateLegacy(

// Register the rest of the types (platform is first because it's needed by others)
registerPlatformTypes(context, serviceManager, isDevMode);
registerTelemetryTypes(serviceManager);
registerNotebookTypes(serviceManager);
registerKernelTypes(serviceManager, isDevMode);
registerInteractiveTypes(serviceManager);
registerIntellisenseTypes(serviceManager, isDevMode);

// Load the two data science experiments that we need to register types
// Await here to keep the register method sync
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import {
isPythonKernelConnection,
getKernelConnectionLanguage,
getLanguageInNotebookMetadata
} from '../kernels/helpers.node';
} from '../kernels/helpers';
import { KernelConnectionMetadata } from '../kernels/types';
import { isJupyterNotebook, getNotebookMetadata } from '../notebooks/helpers.node';
import { translateKernelLanguageToMonaco } from '../platform/common/utils.node';
import { isJupyterNotebook, getNotebookMetadata } from '../notebooks/helpers';
import { translateKernelLanguageToMonaco } from '../platform/common/utils';

export const LastSavedNotebookCellLanguage = 'DATASCIENCE.LAST_SAVED_CELL_LANGUAGE';
/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { IDisposable } from '@fluentui/react';
import { injectable, inject } from 'inversify';
import {
CancellationToken,
Expand Down Expand Up @@ -30,10 +29,10 @@ import { IExtensionSyncActivationService } from '../platform/activation/types';
import { IVSCodeNotebook, IDocumentManager } from '../platform/common/application/types';
import { PYTHON_LANGUAGE } from '../platform/common/constants';
import { disposeAllDisposables } from '../platform/common/helpers';
import { IDisposableRegistry } from '../platform/common/types';
import { IDisposable, IDisposableRegistry } from '../platform/common/types';
import { DataScience } from '../platform/common/utils/localize';
import { JupyterNotebookView } from '../notebooks/constants';
import { getAssociatedJupyterNotebook } from '../notebooks/helpers.node';
import { getAssociatedJupyterNotebook } from '../notebooks/helpers';

type CellUri = string;
type CellVersion = number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import { PYTHON_LANGUAGE } from '../platform/common/constants';
import { traceError } from '../platform/logging';
import { IDisposableRegistry } from '../platform/common/types';
import { noop } from '../platform/common/utils/misc';
import { chainWithPendingUpdates } from '../notebooks/execution/notebookUpdater.node';
import { isJupyterNotebook } from '../notebooks/helpers.node';
import { chainWithPendingUpdates } from '../notebooks/execution/notebookUpdater';
import { isJupyterNotebook } from '../notebooks/helpers';
import { INotebookControllerManager } from '../notebooks/types';
import { translateKernelLanguageToMonaco } from '../platform/common/utils.node';
import { translateKernelLanguageToMonaco } from '../platform/common/utils';
import { IVSCodeNotebookController } from '../notebooks/controllers/types';
/**
* If user creates a blank notebook, then they'll mostl likely end up with a blank cell with language, lets assume `Python`.
Expand Down
27 changes: 12 additions & 15 deletions src/intellisense/intellisenseProvider.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import { IExtensionSyncActivationService } from '../platform/activation/types';
import { IPythonExtensionChecker } from '../platform/api/types';
import { IVSCodeNotebook, IWorkspaceService } from '../platform/common/application/types';
import { IDisposableRegistry, IConfigurationService, IsPreRelease } from '../platform/common/types';
import { IInterpreterService } from '../platform/interpreter/contracts.node';
import { IInterpreterService } from '../platform/interpreter/contracts';
import { PythonEnvironment } from '../platform/pythonEnvironments/info';
import { getInterpreterId } from '../platform/pythonEnvironments/info/interpreter.node';
import { isJupyterNotebook, findAssociatedNotebookDocument } from '../notebooks/helpers.node';
import { getInterpreterId } from '../platform/pythonEnvironments/info/interpreter';
import { isJupyterNotebook, findAssociatedNotebookDocument } from '../notebooks/helpers';
import { INotebookLanguageClientProvider, INotebookControllerManager } from '../notebooks/types';
import { LanguageServer } from './languageServer.node';
import { IInteractiveWindowProvider } from '../interactive-window/types';
Expand Down Expand Up @@ -73,19 +73,18 @@ export class IntellisenseProvider implements INotebookLanguageClientProvider, IE
}

private handleInterpreterChange() {
const folders = [...this.activeInterpreterCache.keys()];
this.activeInterpreterCache.clear();
folders.forEach((f) => this.getActiveInterpreterSync(f));
this.getActiveInterpreterSync(undefined);
}

private getActiveInterpreterSync(fsPath: string | undefined): PythonEnvironment | undefined {
private getActiveInterpreterSync(uri: Uri | undefined): PythonEnvironment | undefined {
if (!this.extensionChecker.isPythonExtensionInstalled) {
return;
}
const folder =
this.workspaceService.getWorkspaceFolder(fsPath ? Uri.file(fsPath) : undefined)?.uri ||
(this.workspaceService.rootPath ? Uri.file(this.workspaceService.rootPath) : undefined);
const key = folder ? folder.fsPath : EmptyWorkspaceKey;
this.workspaceService.getWorkspaceFolder(uri)?.uri ||
(this.workspaceService.rootFolder ? this.workspaceService.rootFolder : undefined);
const key = folder ? getComparisonKey(folder) : EmptyWorkspaceKey;
if (!this.activeInterpreterCache.has(key)) {
this.interpreterService
.getActiveInterpreter(folder)
Expand All @@ -105,7 +104,7 @@ export class IntellisenseProvider implements INotebookLanguageClientProvider, IE
const oldController = this.knownControllers.get(e.notebook);
const oldInterpreter = oldController
? oldController.connection.interpreter
: this.getActiveInterpreterSync(e.notebook.uri.fsPath);
: this.getActiveInterpreterSync(e.notebook.uri);
const oldInterpreterId = oldInterpreter ? this.getInterpreterIdFromCache(oldInterpreter) : undefined;
const oldLanguageServer = oldInterpreterId ? await this.servers.get(oldInterpreterId) : undefined;

Expand Down Expand Up @@ -134,7 +133,7 @@ export class IntellisenseProvider implements INotebookLanguageClientProvider, IE
}

// Make sure the active interpreter cache is up to date
this.getActiveInterpreterSync(n.uri.fsPath);
this.getActiveInterpreterSync(n.uri);

// If the controller is empty, default to the active interpreter
const interpreter =
Expand Down Expand Up @@ -171,9 +170,7 @@ export class IntellisenseProvider implements INotebookLanguageClientProvider, IE
const controller = notebook
? this.notebookControllerManager.getSelectedNotebookController(notebook)
: undefined;
const notebookInterpreter = controller
? controller.connection.interpreter
: this.getActiveInterpreterSync(uri.fsPath);
const notebookInterpreter = controller ? controller.connection.interpreter : this.getActiveInterpreterSync(uri);
let notebookId = notebookInterpreter ? this.getInterpreterIdFromCache(notebookInterpreter) : undefined;

// Special case. For remote use the active interpreter as the controller's interpreter isn't
Expand All @@ -183,7 +180,7 @@ export class IntellisenseProvider implements INotebookLanguageClientProvider, IE
(controller?.connection.kind === 'startUsingRemoteKernelSpec' ||
controller?.connection.kind === 'connectToLiveRemoteKernel')
) {
const activeInterpreter = this.getActiveInterpreterSync(uri.fsPath);
const activeInterpreter = this.getActiveInterpreterSync(uri);
notebookId = activeInterpreter ? this.getInterpreterIdFromCache(activeInterpreter) : undefined;
}

Expand Down
10 changes: 6 additions & 4 deletions src/intellisense/languageServer.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ import { createNotebookMiddleware, createPylanceMiddleware, NotebookMiddleware }
import * as uuid from 'uuid/v4';
import { NOTEBOOK_SELECTOR, PYTHON_LANGUAGE } from '../platform/common/constants';
import { traceInfo } from '../platform/logging';
import { getInterpreterId } from '../platform/pythonEnvironments/info/interpreter.node';
import { getInterpreterId } from '../platform/pythonEnvironments/info/interpreter';
import { noop } from '../platform/common/utils/misc';
import { sleep } from '../platform/common/utils/async';
import { PythonEnvironment } from '../platform/pythonEnvironments/info';
import { getFilePath } from '../platform/common/platform/fs-paths';

// eslint-disable-next-line @typescript-eslint/no-explicit-any
function ensure(target: any, key: string) {
Expand Down Expand Up @@ -134,14 +135,14 @@ export class LanguageServer implements Disposable {
() => languageClient,
() => noop, // Don't trace output. Slows things down too much
NOTEBOOK_SELECTOR,
interpreter.uri.fsPath || interpreter.uri.path,
getFilePath(interpreter.uri),
(uri) => shouldAllowIntellisense(uri, interpreterId, interpreter.uri),
getNotebookHeader
)
: createPylanceMiddleware(
() => languageClient,
NOTEBOOK_SELECTOR,
interpreter.uri.fsPath || interpreter.uri.path,
getFilePath(interpreter.uri),
(uri) => shouldAllowIntellisense(uri, interpreterId, interpreter.uri),
getNotebookHeader
);
Expand Down Expand Up @@ -227,8 +228,9 @@ export class LanguageServer implements Disposable {
if (python) {
const runJediPath = path.join(python.extensionPath, 'pythonFiles', 'run-jedi-language-server.py');
if (await fs.pathExists(runJediPath)) {
const interpreterPath = getFilePath(interpreter.uri);
const serverOptions: ServerOptions = {
command: interpreter.uri.fsPath || 'python',
command: interpreterPath.length > 0 ? interpreterPath : 'python',
args: [runJediPath]
};
return serverOptions;
Expand Down
4 changes: 2 additions & 2 deletions src/intellisense/pythonKernelCompletionProvider.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ import {
} from 'vscode';
import * as lsp from 'vscode-languageclient';
import { IVSCodeNotebook } from '../platform/common/application/types';
import { createPromiseFromCancellation } from '../platform/common/cancellation.node';
import { createPromiseFromCancellation } from '../platform/common/cancellation';
import { traceError, traceInfoIfCI, traceVerbose } from '../platform/logging';
import { getDisplayPath } from '../platform/common/platform/fs-paths';
import { IConfigurationService, IDisposableRegistry } from '../platform/common/types';
import { waitForPromise } from '../platform/common/utils/async';
import { isNotebookCell } from '../platform/common/utils/misc';
import { StopWatch } from '../platform/common/utils/stopWatch';
import { IJupyterSession, IKernelProvider } from '../kernels/types';
import { findAssociatedNotebookDocument, getAssociatedJupyterNotebook } from '../notebooks/helpers.node';
import { findAssociatedNotebookDocument, getAssociatedJupyterNotebook } from '../notebooks/helpers';
import { INotebookLanguageClientProvider } from '../notebooks/types';
import { mapJupyterKind } from './conversion.node';
import { IInteractiveWindowProvider } from '../interactive-window/types';
Expand Down
Loading

0 comments on commit 261afd9

Please sign in to comment.