Skip to content

Commit

Permalink
feat(wrapper): remove dep to @types/vscode (#167)
Browse files Browse the repository at this point in the history
This could cause consuming projects to have multiple versions of
@types/vscode which can cause compliation errors in TSC

BREAKING CHANGE:
`getConfigurations` and `onDidChangeConfiguration` properties
were removed from the `configureLogger` public API
  • Loading branch information
bd82 authored Feb 10, 2021
1 parent d9d9c68 commit b362e5c
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 33 deletions.
6 changes: 2 additions & 4 deletions examples/extension-wrapper/src/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { readFile as readFileCallback } from "fs";
import { promisify } from "util";
import { resolve } from "path";
import { ok } from "assert";
import { ExtensionContext, window, workspace } from "vscode";
import { ExtensionContext, window } from "vscode";
import { IChildLogger, IVSCodeExtLogger } from "@vscode-logging/types";
import { configureLogger, NOOP_LOGGER } from "@vscode-logging/wrapper";

Expand Down Expand Up @@ -47,9 +47,7 @@ export async function initLogger(context: ExtensionContext): Promise<void> {
logConsole: false,
loggingLevelProp: LOGGING_LEVEL_PROP,
sourceLocationProp: SOURCE_LOCATION_PROP,
subscriptions: context.subscriptions,
onDidChangeConfiguration: workspace.onDidChangeConfiguration,
getConfiguration: workspace.getConfiguration
subscriptions: context.subscriptions
});

setLogger(extLogger);
Expand Down
23 changes: 6 additions & 17 deletions packages/wrapper/api.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { workspace, ExtensionContext } from "vscode";
import { IVSCodeExtLogger } from "@vscode-logging/types";
import { BasicOutputChannel } from "@vscode-logging/logger";

Expand All @@ -11,39 +10,29 @@ export declare const NOOP_LOGGER: IVSCodeExtLogger;

export interface ConfigureLoggerOpts {
/**
* @see {GetExtensionLoggerOpts.extName}
* @see {GetExtensionLoggerOpts.extName} in "@vscode-logging/logger"
*/
extName: string;
/**
* @see {GetExtensionLoggerOpts.logPath}
* @see {GetExtensionLoggerOpts.logPath} in "@vscode-logging/logger"
*/
logPath: string;
loggingLevelProp: string;
sourceLocationProp: string;
/**
* @see {GetExtensionLoggerOpts.logOutputChannel}
* @see {GetExtensionLoggerOpts.logOutputChannel} in "@vscode-logging/logger"
*/
logOutputChannel?: BasicOutputChannel;
/**
* @see {GetExtensionLoggerOpts.logConsole}
* @see {GetExtensionLoggerOpts.logConsole} in "@vscode-logging/logger"
*/
logConsole?: boolean;
/**
* The vscode's extension subscriptions
* The vscode extension's subscriptions
* This is normally available via the `activate` function's `context`
* parameter.
*/
subscriptions: ExtensionContext["subscriptions"];
/**
* The `vscode.workspace.getConfiguration` method.
* Note this is the method itself, not the returned value from executing it.
*/
getConfiguration: typeof workspace.getConfiguration;
/**
* The `vscode.workspace.onDidChangeConfiguration` method.
* Note this is the method itself, not the returned value from executing it.
*/
onDidChangeConfiguration: typeof workspace.onDidChangeConfiguration;
subscriptions: { dispose(): any }[];
}

/**
Expand Down
22 changes: 22 additions & 0 deletions packages/wrapper/nyc.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module.exports = {
reporter: ["text", "lcov"],
"check-coverage": false,
all: true,
include: ["dist/src/**/*.js"],
exclude: [
// The `api.ts` is the only file which imports("vscode")
// We could still test it (e.g: with `proxyquire`)
// However the added value of such a test is smaller than
// the overhead cost of implementing such a test:
// - The api.ts only
// 1. Import needed params from `vscode` package.
// 2. merges user arguments with `vscode` params and calls an internal API.
//
// - These merges are partially "tested" at design time by TSC.
// - Implementing a test with proxyquire would not test real VSCode scenarios
// - The existing `configureLogger` tests are fairly complex and we are better off avoiding duplicating them.
// - See: `configure-logger-spec.ts`.
"dist/src/api.js"
],
excludeAfterRemap: false
};
2 changes: 1 addition & 1 deletion packages/wrapper/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
"license": "Apache-2.0",
"typings": "./api.d.ts",
"dependencies": {
"@types/vscode": "1.52.0",
"@vscode-logging/logger": "^1.2.1",
"@vscode-logging/types": "^0.1.3"
},
"devDependencies": {
"@types/vscode": "1.52.0",
"@types/lodash": "^4.14.167",
"lodash": "^4.17.20"
},
Expand Down
25 changes: 24 additions & 1 deletion packages/wrapper/src/api.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,25 @@
export { configureLogger } from "./configure-logger";
/**
* This is the only file in the `src` folder we import("vscode")
* The rest of the code is implemented with Dependency Injection to
* the relevant VSCode APIs to enable easier testing.
*/
import { workspace } from "vscode";
import { configureLoggerInternal } from "./configure-logger";
import { BasicOutputChannel } from "@vscode-logging/logger";
export { NOOP_LOGGER } from "./noop-logger";

export function configureLogger(opts: {
extName: string;
logPath: string;
loggingLevelProp: string;
sourceLocationProp: string;
logOutputChannel?: BasicOutputChannel;
logConsole?: boolean;
subscriptions: { dispose(): any }[];
}) {
return configureLoggerInternal({
...opts,
getConfiguration: workspace.getConfiguration,
onDidChangeConfiguration: workspace.onDidChangeConfiguration
});
}
21 changes: 20 additions & 1 deletion packages/wrapper/src/configure-logger.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { workspace } from "vscode";
import { getExtensionLogger } from "@vscode-logging/logger";
import { IVSCodeExtLogger } from "@vscode-logging/types";
import { ConfigureLoggerOpts } from "../api";
Expand All @@ -10,7 +11,25 @@ import {
logLoggerDetails
} from "./settings-changes-handler";

export function configureLogger(opts: ConfigureLoggerOpts): IVSCodeExtLogger {
export type ConfigureLoggerDIOpts = {
/**
* The `vscode.workspace.getConfiguration` method.
* Note this is the method itself, not the returned value from executing it.
*/
getConfiguration: typeof workspace.getConfiguration;
/**
* The `vscode.workspace.onDidChangeConfiguration` method.
* Note this is the method itself, not the returned value from executing it.
*/
onDidChangeConfiguration: typeof workspace.onDidChangeConfiguration;
};

export type configureLoggerInternalOpts = ConfigureLoggerOpts &
ConfigureLoggerDIOpts;

export function configureLoggerInternal(
opts: configureLoggerInternalOpts
): IVSCodeExtLogger {
const logLevelSetting = getLoggingLevelSetting({
getConfiguration: opts.getConfiguration,
loggingLevelProp: opts.loggingLevelProp
Expand Down
10 changes: 5 additions & 5 deletions packages/wrapper/test/configure-logger-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@ import { expect } from "chai";
import { ExtensionContext, WorkspaceConfiguration } from "vscode";
import { IChildLogger, LogLevel } from "@vscode-logging/types";
import { BasicOutputChannel } from "@vscode-logging/logger";
import { configureLogger } from "../src/api";
import { ConfigureLoggerOpts } from "../api";
import { configureLoggerInternal } from "../src/configure-logger";
import { configureLoggerInternalOpts } from "../src/configure-logger";

describe("The `configureLogger` main wrapper utility function", () => {
let outputChannelMock: BasicOutputChannel;
let subscriptions: ExtensionContext["subscriptions"];
let onDidChangeConfiguration: ConfigureLoggerOpts["onDidChangeConfiguration"];
let onDidChangeConfiguration: configureLoggerInternalOpts["onDidChangeConfiguration"];
let loggingLevelProp: string;
let sourceLocationProp: string;
let configLogLevel: LogLevel;
let getConfiguration: ConfigureLoggerOpts["getConfiguration"];
let getConfiguration: configureLoggerInternalOpts["getConfiguration"];
let loggedLines: Record<string, unknown>[];

beforeEach(() => {
Expand Down Expand Up @@ -47,7 +47,7 @@ describe("The `configureLogger` main wrapper utility function", () => {
});

function configureLoggerHelper(): IChildLogger {
return configureLogger({
return configureLoggerInternal({
extName: "my_vscode_ext",
// only logging "in memory" during our test.
logPath: (undefined as unknown) as string,
Expand Down
4 changes: 4 additions & 0 deletions packages/wrapper/test/noop-logger-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ describe("The no-operation logger", () => {
);
});

it("will not throw when executing the NOOP function", () => {
expect(noop()).to.not.throw;
});

it("implements <getChildLogger> by returning 'itself'", () => {
expect(NOOP_LOGGER.getChildLogger({ label: "foo" })).to.equal(NOOP_LOGGER);
});
Expand Down
7 changes: 3 additions & 4 deletions packages/wrapper/test/settings-changes-handler-spec.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
import { expect } from "chai";
import {
ConfigurationChangeEvent,
ConfigurationScope,
Disposable,
ExtensionContext,
WorkspaceConfiguration
} from "vscode";
import { LogLevel } from "@vscode-logging/logger";
import { IChildLogger, IVSCodeExtLogger } from "@vscode-logging/types";
import { ConfigureLoggerOpts } from "../api";
import { configureLoggerInternalOpts } from "../src/configure-logger";
import { listenToLogSettingsChanges } from "../src/settings-changes-handler";

describe("The `listenToLogSettingsChanges` utility function", () => {
let logger: IVSCodeExtLogger;
let loggingLevelProp: string;
let sourceLocationProp: string;
let logPath: string;
let getConfiguration: ConfigureLoggerOpts["getConfiguration"];
let onDidChangeConfiguration: ConfigureLoggerOpts["onDidChangeConfiguration"];
let getConfiguration: configureLoggerInternalOpts["getConfiguration"];
let onDidChangeConfiguration: configureLoggerInternalOpts["onDidChangeConfiguration"];
let subscriptions: ExtensionContext["subscriptions"];
let currentLogLevel: LogLevel;
let currentSourceLocationTracking: boolean;
Expand Down

0 comments on commit b362e5c

Please sign in to comment.