From 5bc7c9ca04f7d1b753e62dc5f7194c49d4b1f433 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Thu, 12 Dec 2024 10:51:11 -0800 Subject: [PATCH] fix(cli): handle non `@types` imports from tsconfig `compilerOptions.types` (#7548) Synced from https://github.com/aspect-build/aspect-cli/pull/785 by walkerburgin: --- The TypeScript docs for `compilerOptions.types` make it sound like `compilerOptions.types` only applies to `@types` packages, but in practice it can also be used to include types from non `@types` in the global scope. For example, `cypress` recommends this in their [docs](https://docs.cypress.io/app/tooling/typescript-support#Configure-tsconfigjson): ```json { "compilerOptions": { "target": "es5", "lib": ["es5", "dom"], "types": ["cypress", "node"] }, "include": ["**/*.ts"] } ``` From what I can tell, if you don't specify `compilerOptions.types`, TypeScript will **not** automatically include types from a visible non `@types` package like `cypress` (`describe`, `beforeEach`, etc). But you can include non `@types` packages in `compilerOptions.types` to allow list them into the global scope. --- ### Changes are visible to end-users: no ### Test plan I updated the existing test case for `compilerOptions.types`. - Covered by existing test cases - New test cases added Closes aspect-build/aspect-cli#785 Co-authored-by: Walker Burgin GitOrigin-RevId: 4321e2b70b9e8d1f68fff219b42cae02a5d3205a --- gazelle/js/generate.go | 5 +++-- gazelle/js/resolve.go | 2 +- gazelle/js/target.go | 4 ++++ gazelle/js/tests/tsconfig_deps/pnpm-lock.yaml | 2 ++ gazelle/js/tests/tsconfig_deps/types/BUILD.in | 4 ++-- gazelle/js/tests/tsconfig_deps/types/BUILD.out | 5 +++-- gazelle/js/tests/tsconfig_deps/types/tsconfig.json | 2 +- 7 files changed, 16 insertions(+), 8 deletions(-) diff --git a/gazelle/js/generate.go b/gazelle/js/generate.go index e8fa237c4..9d2bd9f87 100644 --- a/gazelle/js/generate.go +++ b/gazelle/js/generate.go @@ -336,14 +336,15 @@ func (ts *typeScriptLang) collectTsConfigImports(cfg *JsGazelleConfig, args lang } for _, t := range tsconfig.Types { - if typesImport := toAtTypesPackage(t); !cfg.IsImportIgnored(typesImport) { + if !cfg.IsImportIgnored(t) { imports = append(imports, ImportStatement{ ImportSpec: resolve.ImportSpec{ Lang: LanguageName, - Imp: typesImport, + Imp: t, }, ImportPath: t, SourcePath: SourcePath, + TypesOnly: true, }) } } diff --git a/gazelle/js/resolve.go b/gazelle/js/resolve.go index ba4164068..ce7033a60 100644 --- a/gazelle/js/resolve.go +++ b/gazelle/js/resolve.go @@ -310,7 +310,7 @@ func (ts *typeScriptLang) resolveImports( deps.Add(typesDep) } - if dep != nil { + if dep != nil && (!imp.TypesOnly || len(types) == 0) { deps.Add(dep) } diff --git a/gazelle/js/target.go b/gazelle/js/target.go index f7b5b5275..336bab54d 100644 --- a/gazelle/js/target.go +++ b/gazelle/js/target.go @@ -23,6 +23,10 @@ type ImportStatement struct { // If the import is optional and failure to resolve should not be an error Optional bool + + // If the import is explicitly for types, in which case prefer @types package + // dependencies when types are shipped separately + TypesOnly bool } // Npm link-all rule import data diff --git a/gazelle/js/tests/tsconfig_deps/pnpm-lock.yaml b/gazelle/js/tests/tsconfig_deps/pnpm-lock.yaml index 7cb0f5a20..da54770bf 100644 --- a/gazelle/js/tests/tsconfig_deps/pnpm-lock.yaml +++ b/gazelle/js/tests/tsconfig_deps/pnpm-lock.yaml @@ -4,10 +4,12 @@ specifiers: '@testing-library/jest-dom': ^5.16.5 '@types/jquery': 3.5.14 '@types/testing-library__jest-dom': ^5.14.5 + cypress: ^13.16.0 jquery: 3.6.1 dependencies: '@testing-library/jest-dom': 5.16.5 '@types/jquery': 3.5.14 '@types/testing-library__jest-dom': 5.14.5 + cypress: 13.16.0 jquery: 3.6.1 diff --git a/gazelle/js/tests/tsconfig_deps/types/BUILD.in b/gazelle/js/tests/tsconfig_deps/types/BUILD.in index bf49273f6..ac8334dd2 100644 --- a/gazelle/js/tests/tsconfig_deps/types/BUILD.in +++ b/gazelle/js/tests/tsconfig_deps/types/BUILD.in @@ -1,2 +1,2 @@ -# gazelle:js_ignore_imports @types/ignored -# gazelle:js_ignore_imports @types/test__ignored +# gazelle:js_ignore_imports ignored +# gazelle:js_ignore_imports @test/ignored diff --git a/gazelle/js/tests/tsconfig_deps/types/BUILD.out b/gazelle/js/tests/tsconfig_deps/types/BUILD.out index 1ea81c2af..b05992a5b 100644 --- a/gazelle/js/tests/tsconfig_deps/types/BUILD.out +++ b/gazelle/js/tests/tsconfig_deps/types/BUILD.out @@ -1,7 +1,7 @@ load("@aspect_rules_ts//ts:defs.bzl", "ts_config") -# gazelle:js_ignore_imports @types/ignored -# gazelle:js_ignore_imports @types/test__ignored +# gazelle:js_ignore_imports ignored +# gazelle:js_ignore_imports @test/ignored ts_config( name = "tsconfig", @@ -9,6 +9,7 @@ ts_config( deps = [ "//:node_modules/@types/jquery", "//:node_modules/@types/testing-library__jest-dom", + "//:node_modules/cypress", "//:tsconfig", ], ) diff --git a/gazelle/js/tests/tsconfig_deps/types/tsconfig.json b/gazelle/js/tests/tsconfig_deps/types/tsconfig.json index 82972d112..c591c04f3 100644 --- a/gazelle/js/tests/tsconfig_deps/types/tsconfig.json +++ b/gazelle/js/tests/tsconfig_deps/types/tsconfig.json @@ -1,6 +1,6 @@ { "extends": "../tsconfig.json", "compilerOptions": { - "types": ["jquery", "@testing-library/jest-dom", "ignored", "@test/ignored"] + "types": ["jquery", "@testing-library/jest-dom", "ignored", "@test/ignored", "cypress"] } }