refactor: Activate on onStartupFinished (#257)

* refactor: Activate on `onStartupFinished`

Closes #214

Stops this extension from slowing VS Code during startup.

* feat: add API, formatter registration notification

Added so that formatter test can wait until formatter is registered
since VS Code has no API to check if a formatter with a given ID exists.

See microsoft/vscode#135674

* docs: add entry to changelog

CHANGELOG.md

@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ## [Unreleased]
 
+### Changed
+
+- The extension no longer blocks VS Code's startup. Thanks to [@robole](https://github.com/robole) for the idea. ([#257](https://github.com/stylelint/vscode-stylelint/pull/257))
+
 ## [1.0.2](https://github.com/stylelint/vscode-stylelint/compare/v1.0.1...v1.0.2) (2021-10-26)
 
 ### Fixed

package.json

@@ -40,7 +40,7 @@
     "check"
   ],
   "activationEvents": [
-    "*"
+    "onStartupFinished"
   ],
   "contributes": {
     "configuration": {
@@ -200,6 +200,7 @@
     "stylelint": "^14.0.0",
     "stylelint-processor-glamorous": "^0.3.0",
     "stylelint-processor-styled-components": "^1.10.0",
+    "typed-emitter": "^1.4.0",
     "typescript": "^4.4.3",
     "zen-observable": "^0.8.15"
   },

src/index.js

@@ -1,19 +1,24 @@
 'use strict';
 
+const events = require('events');
 const {
 	LanguageClient,
 	SettingMonitor,
 	ExecuteCommandRequest,
 } = require('vscode-languageclient/node');
 const { workspace, commands: Commands, window: Window } = require('vscode');
-const { CommandId } = require('./utils/types');
+const { CommandId, Notification, ApiEvent } = require('./utils/types');
 
 /**
  * @param {vscode.ExtensionContext} context
+ * @returns {ExtensionPublicApi}
  */
-exports.activate = ({ subscriptions }) => {
+function activate({ subscriptions }) {
 	const serverPath = require.resolve('./start-server.js');
 
+	/** @type {ExtensionPublicApi} */
+	const api = new events.EventEmitter();
+
 	const client = new LanguageClient(
 		'Stylelint',
 		{
@@ -39,6 +44,12 @@ exports.activate = ({ subscriptions }) => {
 		},
 	);
 
+	client.onReady().then(() => {
+		client.onNotification(Notification.DidRegisterDocumentFormattingEditProvider, () => {
+			api.emit(ApiEvent.DidRegisterDocumentFormattingEditProvider);
+		});
+	});
+
 	subscriptions.push(
 		Commands.registerCommand('stylelint.executeAutofix', async () => {
 			const textEditor = Window.activeTextEditor;
@@ -66,5 +77,12 @@ exports.activate = ({ subscriptions }) => {
 			});
 		}),
 	);
+
 	subscriptions.push(new SettingMonitor(client, 'stylelint.enable').start());
+
+	return api;
+}
+
+module.exports = {
+	activate,
 };

src/server/modules/__tests__/formatter.js

@@ -3,11 +3,13 @@
 const { DocumentFormattingRequest } = require('vscode-languageserver-protocol');
 const { Position, TextEdit } = require('vscode-languageserver-types');
 
+const { Notification } = require('../../../utils/types');
 const { FormatterModule } = require('../formatter');
 
 const mockContext = {
 	connection: {
 		onDocumentFormatting: jest.fn(),
+		sendNotification: jest.fn(),
 		client: { register: jest.fn() },
 	},
 	documents: { get: jest.fn() },
@@ -215,7 +217,7 @@ describe('FormatterModule', () => {
 		);
 	});
 
-	test("with no debug log level, onDidChangeValidateLanguages shouldn't log languages", () => {
+	test("with no debug log level, onDidChangeValidateLanguages shouldn't log languages", async () => {
 		mockLogger.isDebugEnabled.mockReturnValue(false);
 		const module = new FormatterModule(getParams(true));
 
@@ -229,7 +231,7 @@ describe('FormatterModule', () => {
 			}),
 		);
 
-		module.onDidChangeValidateLanguages({
+		await module.onDidChangeValidateLanguages({
 			languages: new Set(['foo']),
 			removedLanguages: new Set(),
 		});
@@ -238,7 +240,7 @@ describe('FormatterModule', () => {
 		expect(mockLogger.debug).not.toHaveBeenCalledWith('Registering formatter for languages');
 	});
 
-	test("without client dynamic registration support, onDidChangeValidateLanguages shouldn't register a formatter", () => {
+	test("without client dynamic registration support, onDidChangeValidateLanguages shouldn't register a formatter", async () => {
 		const module = new FormatterModule(getParams(true));
 
 		module.onInitialize(
@@ -251,15 +253,15 @@ describe('FormatterModule', () => {
 			}),
 		);
 
-		module.onDidChangeValidateLanguages({
+		await module.onDidChangeValidateLanguages({
 			languages: new Set(['foo']),
 			removedLanguages: new Set(),
 		});
 
 		expect(mockContext.connection.client.register).not.toHaveBeenCalled();
 	});
 
-	test('with client dynamic registration support, onDidChangeValidateLanguages should register a formatter', () => {
+	test('with client dynamic registration support, onDidChangeValidateLanguages should register a formatter', async () => {
 		const module = new FormatterModule(getParams(true));
 
 		module.onInitialize(
@@ -272,7 +274,7 @@ describe('FormatterModule', () => {
 			}),
 		);
 
-		module.onDidChangeValidateLanguages({
+		await module.onDidChangeValidateLanguages({
 			languages: new Set(['foo']),
 			removedLanguages: new Set(),
 		});
@@ -283,7 +285,7 @@ describe('FormatterModule', () => {
 		);
 	});
 
-	test('without languages to validate, onDidChangeValidateLanguages should register a formatter', () => {
+	test('when a formatter is registered, a notification should be sent', async () => {
 		const module = new FormatterModule(getParams(true));
 
 		module.onInitialize(
@@ -296,23 +298,43 @@ describe('FormatterModule', () => {
 			}),
 		);
 
-		module.onDidChangeValidateLanguages({
+		await module.onDidChangeValidateLanguages({
+			languages: new Set(['foo']),
+			removedLanguages: new Set(),
+		});
+
+		expect(mockContext.connection.sendNotification).toHaveBeenCalledWith(
+			Notification.DidRegisterDocumentFormattingEditProvider,
+			{},
+		);
+	});
+
+	test('without languages to validate, onDidChangeValidateLanguages should register a formatter', async () => {
+		const module = new FormatterModule(getParams(true));
+
+		module.onInitialize(
+			/** @type {any} */ ({
+				capabilities: {
+					textDocument: {
+						formatting: { dynamicRegistration: true },
+					},
+				},
+			}),
+		);
+
+		await module.onDidChangeValidateLanguages({
 			languages: new Set(),
 			removedLanguages: new Set(),
 		});
 
 		expect(mockContext.connection.client.register).not.toHaveBeenCalled();
 	});
 
-	test('when a formatter was already registered, onDidChangeValidateLanguages should dispose the old registration', () => {
+	test('when a formatter was already registered, onDidChangeValidateLanguages should dispose the old registration', async () => {
 		mockLogger.isDebugEnabled.mockReturnValue(true);
-		const fakePromise = (/** @type {any} */ resolutionValue) => ({
-			then: (/** @type {Function} */ resolve) => resolve(resolutionValue),
-		});
-
-		const mockRegistration = { dispose: jest.fn(() => fakePromise()) };
+		const mockRegistration = { dispose: jest.fn() };
 
-		mockContext.connection.client.register.mockReturnValueOnce(fakePromise(mockRegistration));
+		mockContext.connection.client.register.mockResolvedValueOnce(mockRegistration);
 
 		const module = new FormatterModule(getParams(true));
 
@@ -326,12 +348,12 @@ describe('FormatterModule', () => {
 			}),
 		);
 
-		module.onDidChangeValidateLanguages({
+		await module.onDidChangeValidateLanguages({
 			languages: new Set(['foo']),
 			removedLanguages: new Set(),
 		});
 
-		module.onDidChangeValidateLanguages({
+		await module.onDidChangeValidateLanguages({
 			languages: new Set(['bar']),
 			removedLanguages: new Set(['foo']),
 		});

src/server/modules/formatter.js

@@ -2,6 +2,7 @@
 
 const { DocumentFormattingRequest } = require('vscode-languageserver-protocol');
 const { formattingOptionsToRules } = require('../../utils/stylelint');
+const { Notification } = require('../../utils/types');
 
 /**
  * @implements {LanguageServerModule}
@@ -29,9 +30,8 @@ class FormatterModule {
 	#registerDynamically = false;
 
 	/**
-	 * A promise that resolves to the disposable for the dynamically registered
-	 * document formatter.
-	 * @type {Promise<lsp.Disposable> | undefined}
+	 * The disposable for the dynamically registered document formatter.
+	 * @type {lsp.Disposable | undefined}
 	 */
 	#registration = undefined;
 
@@ -120,9 +120,9 @@ class FormatterModule {
 
 	/**
 	 * @param {DidChangeValidateLanguagesParams} params
-	 * @returns {void}
+	 * @returns {Promise<void>}
 	 */
-	onDidChangeValidateLanguages({ languages }) {
+	async onDidChangeValidateLanguages({ languages }) {
 		if (this.#logger?.isDebugEnabled()) {
 			this.#logger?.debug('Received onDidChangeValidateLanguages', { languages: [...languages] });
 		}
@@ -134,11 +134,9 @@ class FormatterModule {
 			if (this.#registration) {
 				this.#logger?.debug('Disposing old formatter registration');
 
-				void this.#registration
-					.then((disposable) => disposable.dispose())
-					.then(() => {
-						this.#logger?.debug('Old formatter registration disposed');
-					});
+				this.#registration.dispose();
+
+				this.#logger?.debug('Old formatter registration disposed');
 			}
 
 			// If there are languages that should be validated, register a formatter for those
@@ -154,11 +152,16 @@ class FormatterModule {
 					this.#logger?.debug('Registering formatter for languages', { languages: [...languages] });
 				}
 
-				this.#registration = this.#context.connection.client.register(
+				this.#registration = await this.#context.connection.client.register(
 					DocumentFormattingRequest.type,
 					{ documentSelector },
 				);
 
+				this.#context.connection.sendNotification(
+					Notification.DidRegisterDocumentFormattingEditProvider,
+					{},
+				);
+
 				this.#logger?.debug('Formatter registered');
 			}
 		}

src/utils/types.js

@@ -29,6 +29,25 @@ const DisableReportRuleNames = {
 	Illegal: 'reportDisables',
 };
 
+/**
+ * Language server notification names.
+ * @enum {string}
+ */
+const Notification = {
+	DidRegisterDocumentFormattingEditProvider:
+		'textDocument/didRegisterDocumentFormattingEditProvider',
+};
+
+/**
+ * Extension API event names.
+ * @enum {keyof ExtensionEvents}
+ */
+const ApiEvent = {
+	DidRegisterDocumentFormattingEditProvider: /** @type {keyof ExtensionEvents} */ (
+		'DidRegisterDocumentFormattingEditProvider'
+	),
+};
+
 /**
  * Error thrown when a rule's option is invalid.
  */
@@ -46,5 +65,7 @@ module.exports = {
 	CommandId,
 	CodeActionKind,
 	DisableReportRuleNames,
+	Notification,
+	ApiEvent,
 	InvalidOptionError,
 };

test/e2e/formatter/index.js

@@ -2,9 +2,35 @@
 
 const path = require('path');
 
-const { workspace, commands, window } = require('vscode');
+const { workspace, commands, window, extensions } = require('vscode');
+const { ApiEvent } = require('../../../src/utils/types');
 
 describe('vscode-stylelint', () => {
+	beforeAll(async () => {
+		const extension = extensions.getExtension('stylelint.vscode-stylelint');
+
+		if (!extension) {
+			throw new Error('Unable to find Stylelint extension');
+		}
+
+		const api = /** @type {ExtensionPublicApi} */ (extension.exports);
+
+		await /** @type {Promise<void>} */ (
+			new Promise((resolve, reject) => {
+				const timeout = setTimeout(
+					() =>
+						reject(new Error('Did not receive DidRegisterDocumentFormattingEditProvider event')),
+					2000,
+				);
+
+				api.on(ApiEvent.DidRegisterDocumentFormattingEditProvider, () => {
+					clearTimeout(timeout);
+					resolve();
+				});
+			})
+		);
+	});
+
 	it('should format document using formatting options', async () => {
 		// Open the './test.css' file.
 		const cssDocument = await workspace.openTextDocument(path.resolve(__dirname, 'test.css'));