Skip to content

Commit

Permalink
fix(cli): handle non @types imports from tsconfig `compilerOptions.…
Browse files Browse the repository at this point in the history
…types` (#7548)

Synced from #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 #785

Co-authored-by: Walker Burgin <wburgin@anduril.com>
GitOrigin-RevId: 4321e2b70b9e8d1f68fff219b42cae02a5d3205a
  • Loading branch information
jbedard and walkerburgin committed Dec 13, 2024
1 parent 8f91b83 commit 864c2f2
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 8 deletions.
5 changes: 3 additions & 2 deletions gazelle/js/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion gazelle/js/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ func (ts *typeScriptLang) resolveImports(
deps.Add(typesDep)
}

if dep != nil {
if dep != nil && (!imp.TypesOnly || len(types) == 0) {
deps.Add(dep)
}

Expand Down
4 changes: 4 additions & 0 deletions gazelle/js/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions gazelle/js/tests/tsconfig_deps/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions gazelle/js/tests/tsconfig_deps/types/BUILD.in
Original file line number Diff line number Diff line change
@@ -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
5 changes: 3 additions & 2 deletions gazelle/js/tests/tsconfig_deps/types/BUILD.out
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
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",
src = "tsconfig.json",
deps = [
"//:node_modules/@types/jquery",
"//:node_modules/@types/testing-library__jest-dom",
"//:node_modules/cypress",
"//:tsconfig",
],
)
2 changes: 1 addition & 1 deletion gazelle/js/tests/tsconfig_deps/types/tsconfig.json
Original file line number Diff line number Diff line change
@@ -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"]
}
}

0 comments on commit 864c2f2

Please sign in to comment.