Upgrade to using shared utils

- better dependency management
- use full completion result when using suggestions

Author: apexskier

package.json

@@ -27,6 +27,7 @@
   },
   "homepage": "https://github.com/apexskier/nova-typescript",
   "devDependencies": {
+    "@rollup/plugin-commonjs": "^15.1.0",
     "@rollup/plugin-node-resolve": "^9.0.0",
     "@types/jest": "^26.0.14",
     "@types/nova-editor-node": "^1.0.4",
@@ -36,6 +37,7 @@
     "eslint": "^7.10.0",
     "eslint-plugin-nova": "^1.0.0",
     "jest": "^26.4.2",
+    "nova-extension-utils": "^1.0.2",
     "onchange": "^7.0.2",
     "prettier": "^2.1.2",
     "rollup": "^2.28.2",

rollup.config.main.js

@@ -1,20 +1,10 @@
 import typescript from "rollup-plugin-typescript2";
+import commonjs from "@rollup/plugin-commonjs";
 import resolve from "@rollup/plugin-node-resolve";
 
-// NOTE: this works, but has a couple issues.
-//
-// cache issues, such as the following
-// ```
-// [!] (plugin rpt2) Error: ENOENT: no such file or directory, open '/Users/cameronlittle/Dev/nova-typescript/node_modules/.cache/rollup-plugin-typescript2/rpt2_b2ffe25927fdf3bdfc6af7cc34cbf7c51560889e/code/cache_/dff2aa675eb1f7c9d04145fc75469e189b910ce5'
-// ```
-//
-// it doesn't use the same type resolution as tsc, which has conflicts with @types/node
-//
-// I'm going to wait until my nova-editor types PR is merged before attempting to fix fully
-
 export default {
   input: "src/main.ts",
-  plugins: [typescript(), resolve()],
+  plugins: [typescript(), commonjs(), resolve()],
   output: {
     file: "typescript.novaextension/Scripts/main.dist.js",
     sourcemap: true,

src/commands/autoSuggest.ts

@@ -1,7 +1,8 @@
 // eslint-disable-next-line no-unused-vars
 import type * as lspTypes from "vscode-languageserver-protocol";
+import { asyncNova } from "nova-extension-utils";
 import { rangeToLspRange, lspRangeToRange } from "../lspNovaConversions";
-import { wrapCommand, showChoicePalette } from "../novaUtils";
+import { wrapCommand } from "../novaUtils";
 import { executeCommand } from "./codeAction";
 
 export function registerAutoSuggest(client: LanguageClient) {
@@ -50,7 +51,7 @@ export function registerAutoSuggest(client: LanguageClient) {
       return;
     }
 
-    const choice = await showChoicePalette(
+    let choice = await asyncNova.showChoicePalette(
       items,
       (item) => `${item.label}${item.detail ? `- ${item.detail}` : ""}`,
       { placeholder: "suggestions" }
@@ -59,13 +60,13 @@ export function registerAutoSuggest(client: LanguageClient) {
       return;
     }
 
-    const completionItem = (await client.sendRequest(
+    choice = (await client.sendRequest(
       "completionItem/resolve",
       choice
     )) as lspTypes.CompletionItem;
 
     if (nova.inDevMode()) {
-      console.log(JSON.stringify(completionItem, null, "  "));
+      console.log(JSON.stringify(choice, null, "  "));
     }
 
     const { textEdit, additionalTextEdits, command } = choice;

src/commands/codeAction.ts

@@ -1,8 +1,9 @@
 // eslint-disable-next-line no-unused-vars
 import type * as lspTypes from "vscode-languageserver-protocol";
 import * as lsp from "vscode-languageserver-types";
+import { asyncNova } from "nova-extension-utils";
 import { applyWorkspaceEdit } from "../applyWorkspaceEdit";
-import { wrapCommand, showChoicePalette } from "../novaUtils";
+import { wrapCommand } from "../novaUtils";
 import { rangeToLspRange } from "../lspNovaConversions";
 
 // @Panic: this is totally decoupled from typescript, so it could totally be native to Nova
@@ -43,7 +44,7 @@ export function registerCodeAction(client: LanguageClient) {
       return;
     }
 
-    const choice = await showChoicePalette(response, (c) => c.title, {
+    const choice = await asyncNova.showChoicePalette(response, (c) => c.title, {
       placeholder: "Choose an action",
     });
     if (choice == null) {
@@ -77,7 +78,6 @@ export async function executeCommand(
   client: LanguageClient,
   command: lspTypes.Command
 ) {
-  console.info("executing command", command.command);
   const params: lspTypes.ExecuteCommandParams = {
     command: command.command,
     arguments: command.arguments,

src/main.test.ts

@@ -7,6 +7,7 @@ jest.mock("./tsLibPath", () => ({
 jest.mock("./isEnabledForJavascript", () => ({
   isEnabledForJavascript: () => true,
 }));
+jest.mock("nova-extension-utils");
 
 jest.useFakeTimers();
 
@@ -59,6 +60,12 @@ describe("test suite", () => {
   const { activate, deactivate } = require("./main");
 
   function resetMocks() {
+    const {
+      dependencyManagement: { installWrappedDependencies },
+    } = require("nova-extension-utils");
+    installWrappedDependencies
+      .mockReset()
+      .mockImplementation(() => Promise.resolve());
     nova.fs.access = jest.fn().mockReturnValue(true);
     (nova.commands.register as jest.Mock).mockReset();
     LanguageClientMock.mockReset().mockImplementation(() => ({
@@ -95,6 +102,14 @@ describe("test suite", () => {
     );
 
     expect(CompositeDisposable).toBeCalledTimes(1);
+
+    const {
+      registerDependencyUnlockCommand,
+    } = require("nova-extension-utils").dependencyManagement;
+    expect(registerDependencyUnlockCommand).toBeCalledTimes(1);
+    expect(registerDependencyUnlockCommand).toBeCalledWith(
+      "apexskier.typescript.forceClearLock"
+    );
   });
 
   function assertActivationBehavior() {
@@ -124,22 +139,20 @@ describe("test suite", () => {
       expect.any(Function)
     );
 
-    expect(Process).toBeCalledTimes(3);
     // installs dependencies
-    expect(Process).toHaveBeenNthCalledWith(1, "/usr/bin/env", {
-      args: ["npm", "install"],
-      cwd: "/extension",
-      stdio: ["ignore", "pipe", "pipe"],
-      env: {
-        NO_UPDATE_NOTIFIER: "true",
-      },
-    });
+
+    const {
+      dependencyManagement: { installWrappedDependencies },
+    } = require("nova-extension-utils");
+    expect(installWrappedDependencies).toBeCalledTimes(1);
+
+    expect(Process).toBeCalledTimes(2);
     // makes the run script executable
-    expect(Process).toHaveBeenNthCalledWith(2, "/usr/bin/env", {
+    expect(Process).toHaveBeenNthCalledWith(1, "/usr/bin/env", {
       args: ["chmod", "u+x", "/extension/run.sh"],
     });
     // gets the typescript version
-    expect(Process).toHaveBeenNthCalledWith(3, "/usr/bin/env", {
+    expect(Process).toHaveBeenNthCalledWith(2, "/usr/bin/env", {
       args: ["node", "/tsLibPath/tsc.js", "--version"],
       stdio: ["ignore", "pipe", "ignore"],
     });
@@ -168,15 +181,6 @@ describe("test suite", () => {
       resetMocks();
 
       (ProcessMock as jest.Mock<Partial<Process>>)
-        .mockImplementationOnce(() => ({
-          onStdout: jest.fn(),
-          onStderr: jest.fn(),
-          onDidExit: jest.fn((cb) => {
-            cb(0);
-            return { dispose: jest.fn() };
-          }),
-          start: jest.fn(),
-        }))
         .mockImplementationOnce(() => ({
           onStdout: jest.fn(),
           onStderr: jest.fn(),
@@ -205,7 +209,7 @@ describe("test suite", () => {
         informationViewModule.InformationView
       >).mock.instances[0];
       expect(informationView.tsVersion).toBeUndefined();
-      const tsVersionProcess: Process = ProcessMock.mock.results[2].value;
+      const tsVersionProcess: Process = ProcessMock.mock.results[1].value;
       const exitCB = (tsVersionProcess.onDidExit as jest.Mock).mock.calls[0][0];
       exitCB(0);
       // allow promise to execute
@@ -228,19 +232,11 @@ describe("test suite", () => {
       global.console.warn = jest.fn();
       nova.workspace.showErrorMessage = jest.fn();
 
-      (ProcessMock as jest.Mock<Partial<Process>>).mockImplementationOnce(
-        () => ({
-          onStdout: jest.fn(),
-          onStderr: jest.fn((cb) => {
-            cb("some output on stderr");
-            return { dispose: jest.fn() };
-          }),
-          onDidExit: jest.fn((cb) => {
-            cb(1);
-            return { dispose: jest.fn() };
-          }),
-          start: jest.fn(),
-        })
+      const {
+        dependencyManagement: { installWrappedDependencies },
+      } = require("nova-extension-utils");
+      installWrappedDependencies.mockImplementation(() =>
+        Promise.reject(new Error("Failed to install:\n\nsome output on stderr"))
       );
 
       await activate();

src/main.ts

@@ -1,3 +1,4 @@
+import { dependencyManagement } from "nova-extension-utils";
 import { registerAutoSuggest } from "./commands/autoSuggest";
 import { registerCodeAction } from "./commands/codeAction";
 import { registerFindReferences } from "./commands/findReferences";
@@ -14,44 +15,19 @@ import { isEnabledForJavascript } from "./isEnabledForJavascript";
 nova.commands.register(
   "apexskier.typescript.openWorkspaceConfig",
   wrapCommand(function openWorkspaceConfig(workspace: Workspace) {
-    workspace.openConfig("apexskier.typescript");
+    workspace.openConfig();
   })
 );
 
 nova.commands.register("apexskier.typescript.reload", reload);
 
+dependencyManagement.registerDependencyUnlockCommand(
+  "apexskier.typescript.forceClearLock"
+);
+
 let client: LanguageClient | null = null;
 const compositeDisposable = new CompositeDisposable();
 
-async function installWrappedDependencies() {
-  return new Promise((resolve, reject) => {
-    const process = new Process("/usr/bin/env", {
-      args: ["npm", "install"],
-      cwd: nova.extension.path,
-      stdio: ["ignore", "pipe", "pipe"],
-      env: {
-        NO_UPDATE_NOTIFIER: "true",
-      },
-    });
-    let errOutput = "";
-    if (nova.inDevMode()) {
-      process.onStdout((o) => console.log("installing:", o.trimRight()));
-    }
-    process.onStderr((e) => {
-      console.warn("installing:", e.trimRight());
-      errOutput += e;
-    });
-    process.onDidExit((status) => {
-      if (status === 0) {
-        resolve();
-      } else {
-        reject(new Error(`Failed to install:\n\n${errOutput}`));
-      }
-    });
-    process.start();
-  });
-}
-
 async function makeFileExecutable(file: string) {
   return new Promise((resolve, reject) => {
     const process = new Process("/usr/bin/env", {
@@ -102,7 +78,7 @@ async function asyncActivate() {
   informationView.status = "Activating...";
 
   try {
-    await installWrappedDependencies();
+    await dependencyManagement.installWrappedDependencies(compositeDisposable);
   } catch (err) {
     informationView.status = "Failed to install";
     throw err;
@@ -132,8 +108,8 @@ async function asyncActivate() {
     });
     console.log("logging to", logDir);
     // passing inLog breaks some requests for an unknown reason
-    // const inLog = nova.path.join(logDir, "languageClient-in.log");
-    const outLog = nova.path.join(logDir, "languageClient-out.log");
+    // const inLog = nova.path.join(logDir, "languageServer-in.log");
+    const outLog = nova.path.join(logDir, "languageServer-out.log");
     serviceArgs = {
       path: "/usr/bin/env",
       // args: ["bash", "-c", `tee "${inLog}" | "${runFile}" | tee "${outLog}"`],
@@ -158,6 +134,7 @@ async function asyncActivate() {
       env: {
         TSLIB_PATH: tslibPath,
         WORKSPACE_DIR: nova.workspace.path ?? "",
+        INSTALL_DIR: dependencyManagement.getDependencyDirectory(),
       },
     },
     {

src/novaUtils.ts

@@ -19,23 +19,3 @@ export async function openFile(uri: string) {
   // try one more time, this doesn't resolve if the file isn't already open. Need to file a bug
   return await nova.workspace.openFile(uri);
 }
-
-export async function showChoicePalette<T>(
-  choices: T[],
-  choiceToString: (choice: T) => string,
-  options?: { placeholder?: string }
-) {
-  const index = await new Promise<number | null>((resolve) =>
-    nova.workspace.showChoicePalette(
-      choices.map(choiceToString),
-      options,
-      (_, index) => {
-        resolve(index);
-      }
-    )
-  );
-  if (index == null) {
-    return null;
-  }
-  return choices[index];
-}

typescript.novaextension/CHANGELOG.md

@@ -1,5 +1,17 @@
 # Changelog
 
+## future
+
+### Changed
+
+- Major behind the scene changes to dependency management
+  - Performance improvements
+  - More reliability with multiple open workspaces
+
+### Fixed
+
+- Use full completion result in manual auto-suggest command
+
 ## v1.7.0
 
 ### Added

typescript.novaextension/extension.json

@@ -108,6 +108,10 @@
       {
         "title": "Find Symbol",
         "command": "apexskier.typescript.findSymbol"
+      },
+      {
+        "title": "Force Unlock Dependency Installation",
+        "command": "apexskier.typescript.forceClearLock"
       }
     ],
     "editor": [

typescript.novaextension/run.sh

@@ -1,26 +1,20 @@
 #!/usr/bin/env bash
 
-DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"
-
-# env > /dev/stderr
-# echo $PATH > /dev/stderr
-# dirname $(command -v node) > /dev/stderr
-# note: any output from this script will be used by nova's language server thing, so it'll break functionality
+# note: any output from this script will be used by nova's language server, so it'll break functionality
 
 cd "$WORKSPACE_DIR"
 
 # note: --tsserver-path=".../tsserver.js" doesn't support debugging since it tries to fork and bind two processes to the same port
 
-# path is stripped in the extension execution environment somehow
 # symlinks have issues when the extension is submitted to the library, so we don't use node_modules/.bin
 node \
-	"$DIR/node_modules/typescript-language-server/lib/cli.js" \
+	"$INSTALL_DIR/node_modules/typescript-language-server/lib/cli.js" \
 	--stdio \
 	--tsserver-path="$TSLIB_PATH/tsserver.js"
 
 # use this for debugging
 # node \
 # 	--inspect-brk \
-# 	"$DIR/../node_modules/typescript-language-server/lib/cli.js" \
+# 	"$INSTALL_DIR/node_modules/typescript-language-server/lib/cli.js" \
 # 	--stdio \
 # 	--tsserver-path="$TSLIB_PATH/tsserver"