cursorless icon indicating copy to clipboard operation
cursorless copied to clipboard

Switch to docstring imports for things we import only for docstrings

Open pokey opened this issue 1 year ago • 0 comments

When we refer to a type in a docstring using something like {@link Foo}, we need Foo to have been imported. Sometimes we will import just for the sake of using it in a docstring, but then we need a lint ignore rule to get typescript not to complain about unused var. See eg https://github.com/cursorless-dev/cursorless/blob/12a289129f5a24a126ef9b1bc6192566aff2267d/packages/cursorless-engine/src/processTargets/modifiers/commonContainingScopeIfUntypedModifiers.ts#L2-L7 There are several of these examples

In Typescript 5.5, there is now support for @import directives in docstrings. See

  • https://devblogs.microsoft.com/typescript/announcing-typescript-5-5/#the-jsdoc-import-tag
  • https://github.com/microsoft/TypeScript/pull/57207

However, this didn't work for me, and the type inference just said any:

image

We should figure out what's going on here, then replace all the ignored imports with this new @import directive. Here is a diff that does it in a couple places, but there are probably more:

From 3064d2f63c0e0a24f8ee8fd6a4eecfc29aa39cac Mon Sep 17 00:00:00 2001
From: Pokey Rule <[email protected]>
Date: Thu, 18 Jul 2024 15:14:43 +0100
Subject: [PATCH] Attempt to use docstring imports

---
 ...commonContainingScopeIfUntypedModifiers.ts |  8 +------
 .../src/typings/target.types.ts               | 23 +++----------------
 2 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/packages/cursorless-engine/src/processTargets/modifiers/commonContainingScopeIfUntypedModifiers.ts b/packages/cursorless-engine/src/processTargets/modifiers/commonContainingScopeIfUntypedModifiers.ts
index 38615f18a..afb7988cc 100644
--- a/packages/cursorless-engine/src/processTargets/modifiers/commonContainingScopeIfUntypedModifiers.ts
+++ b/packages/cursorless-engine/src/processTargets/modifiers/commonContainingScopeIfUntypedModifiers.ts
@@ -1,11 +1,5 @@
+/** @import { Target } from "../../typings/target.types" */
 import { Modifier } from "@cursorless/common";
-// NB: We import `Target` below just so that @link below resolves.  Once one of
-// the following issues are fixed, we can either remove the above line or
-// switch to `{import("foo")}` syntax in the `{@link}` tag.
-// - https://github.com/microsoft/TypeScript/issues/43869
-// - https://github.com/microsoft/TypeScript/issues/43950
-// eslint-disable-next-line @typescript-eslint/no-unused-vars, unused-imports/no-unused-imports
-import type { Target } from "../../typings/target.types";
 
 /**
  * Expands the given target to the nearest containing line if the target has no
diff --git a/packages/cursorless-engine/src/typings/target.types.ts b/packages/cursorless-engine/src/typings/target.types.ts
index 1e54b9ce4..652264a77 100644
--- a/packages/cursorless-engine/src/typings/target.types.ts
+++ b/packages/cursorless-engine/src/typings/target.types.ts
@@ -1,30 +1,13 @@
-// NB: We import `Target` below just so that @link below resolves.  Once one of
-// the following issues are fixed, we can either remove the above line or
-// switch to `{import("foo")}` syntax in the `{@link}` tag.
-// - https://github.com/microsoft/TypeScript/issues/43869
-// - https://github.com/microsoft/TypeScript/issues/43950
-// eslint-disable-next-line @typescript-eslint/no-unused-vars, unused-imports/no-unused-imports
-import type { ModifyIfUntypedStage } from "../processTargets/modifiers/ConditionalModifierStages";
-// eslint-disable-next-line @typescript-eslint/no-unused-vars, unused-imports/no-unused-imports
+/** @import { ModifyIfUntypedStage } from "../processTargets/modifiers/ConditionalModifierStages" */
+/** @import { Snippet, SnippetVariable } from "@cursorless/common" */
+/** @import { ScopeTypeTarget, TokenTarget, UntypedTarget } from "../processTargets/targets" */
 import type {
   InsertionMode,
   Range,
   Selection,
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars, unused-imports/no-unused-imports
-  Snippet,
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars, unused-imports/no-unused-imports
-  SnippetVariable,
   TargetPlainObject,
   TextEditor,
 } from "@cursorless/common";
-import type {
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars, unused-imports/no-unused-imports
-  ScopeTypeTarget,
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars, unused-imports/no-unused-imports
-  TokenTarget,
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars, unused-imports/no-unused-imports
-  UntypedTarget,
-} from "../processTargets/targets";
 import type { EditWithRangeUpdater } from "./Types";
 
 export type EditNewActionType = "edit" | "insertLineAfter";
-- 
2.43.0

pokey avatar Jul 18 '24 14:07 pokey