From 62685bac3eab80c9c77f12bc2ee386d7b5c40b19 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Tue, 8 Oct 2024 16:30:11 -0700 Subject: [PATCH] fix(cli): treat package.json imports as optional dependencies (#7044) There are too many edge cases that are causing issues atm. This should have been optional or opt-in from the start. This makes these imports "optional", so the feature is still enabled by default and users upgrading the cli will get this new feature, but it should no longer ever cause errors. We can maybe change it to error when opted-in in the future and maybe change the default if we are more certain it handles most of the edge cases. --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): yes - Suggested release notes appear below: yes Dependencies from package.json `main/types/exports` fields no longer cause errors when `aspect configure` can not resolve the referenced files. ### Test plan - New test cases added GitOrigin-RevId: 3199133ffd87902ab1906811d5383bb0de56b299 --- gazelle/js/generate.go | 5 ++++- gazelle/js/resolve.go | 4 +++- gazelle/js/target.go | 3 +++ .../npm_package_deps_lib/exports-ignore/package.json | 1 + .../js/tests/npm_package_deps_lib/not-found/BUILD.in | 1 + .../js/tests/npm_package_deps_lib/not-found/BUILD.out | 8 ++++++++ .../tests/npm_package_deps_lib/not-found/package.json | 10 ++++++++++ .../tests/npm_package_deps_lib/not-found/src/main.ts | 0 8 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 gazelle/js/tests/npm_package_deps_lib/not-found/BUILD.in create mode 100644 gazelle/js/tests/npm_package_deps_lib/not-found/BUILD.out create mode 100644 gazelle/js/tests/npm_package_deps_lib/not-found/package.json create mode 100644 gazelle/js/tests/npm_package_deps_lib/not-found/src/main.ts diff --git a/gazelle/js/generate.go b/gazelle/js/generate.go index bb1d6e862..93598d165 100644 --- a/gazelle/js/generate.go +++ b/gazelle/js/generate.go @@ -220,7 +220,7 @@ func (ts *typeScriptLang) addPackageRule(cfg *JsGazelleConfig, args language.Gen npmPackageInfo.sources.Add(impt) } else { if strings.Contains(impt, "*") { - BazelLog.Warnf("Wildcard import %q in %q not supported", impt, packageJsonPath) + BazelLog.Debugf("Wildcard import %q in %q not supported", impt, packageJsonPath) continue } @@ -231,6 +231,9 @@ func (ts *typeScriptLang) addPackageRule(cfg *JsGazelleConfig, args language.Gen }, ImportPath: impt, SourcePath: packageJsonPath, + + // Set as optional while package.json imports are experimental + Optional: true, }) } } diff --git a/gazelle/js/resolve.go b/gazelle/js/resolve.go index a40190a34..645fc0804 100644 --- a/gazelle/js/resolve.go +++ b/gazelle/js/resolve.go @@ -305,7 +305,9 @@ func (ts *typeScriptLang) resolveImports( // Neither the import or a type definition was found. if resolutionType == Resolution_NotFound && len(types) == 0 { - if cfg.ValidateImportStatements() != ValidationOff { + if imp.Optional { + BazelLog.Infof("Optional import %q for target %q not found", imp.ImportPath, from.String()) + } else if cfg.ValidateImportStatements() != ValidationOff { BazelLog.Debugf("import %q for target %q not found", imp.ImportPath, from.String()) notFound := fmt.Errorf( diff --git a/gazelle/js/target.go b/gazelle/js/target.go index 904063132..cf768c542 100644 --- a/gazelle/js/target.go +++ b/gazelle/js/target.go @@ -21,6 +21,9 @@ type ImportStatement struct { // The path as written in the import statement ImportPath string + + // If the import is optional and failure to resolve should not be an error + Optional bool } // Npm link-all rule import data diff --git a/gazelle/js/tests/npm_package_deps_lib/exports-ignore/package.json b/gazelle/js/tests/npm_package_deps_lib/exports-ignore/package.json index acdcfbc78..4d074a203 100644 --- a/gazelle/js/tests/npm_package_deps_lib/exports-ignore/package.json +++ b/gazelle/js/tests/npm_package_deps_lib/exports-ignore/package.json @@ -7,6 +7,7 @@ }, "./bin": "./bin/does-not-exist.js", "./dne": "./does-not-exist.js", + "./not-ignored-dne": "./not-ignored.js", "./lib": "./lib1.js" } } diff --git a/gazelle/js/tests/npm_package_deps_lib/not-found/BUILD.in b/gazelle/js/tests/npm_package_deps_lib/not-found/BUILD.in new file mode 100644 index 000000000..066d7e736 --- /dev/null +++ b/gazelle/js/tests/npm_package_deps_lib/not-found/BUILD.in @@ -0,0 +1 @@ +# gazelle:generation_mode update diff --git a/gazelle/js/tests/npm_package_deps_lib/not-found/BUILD.out b/gazelle/js/tests/npm_package_deps_lib/not-found/BUILD.out new file mode 100644 index 000000000..c245e1e88 --- /dev/null +++ b/gazelle/js/tests/npm_package_deps_lib/not-found/BUILD.out @@ -0,0 +1,8 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") + +# gazelle:generation_mode update + +ts_project( + name = "tsc", + srcs = ["src/main.ts"], +) diff --git a/gazelle/js/tests/npm_package_deps_lib/not-found/package.json b/gazelle/js/tests/npm_package_deps_lib/not-found/package.json new file mode 100644 index 000000000..a7c940ec9 --- /dev/null +++ b/gazelle/js/tests/npm_package_deps_lib/not-found/package.json @@ -0,0 +1,10 @@ +{ + "name": "pkg-with-exports", + "private": true, + "main": "./not-found.js", + "types": "./not-found.d.ts", + "exports": { + ".": "./dist/main.js", + "./foo": "./not-found.js" + } +} diff --git a/gazelle/js/tests/npm_package_deps_lib/not-found/src/main.ts b/gazelle/js/tests/npm_package_deps_lib/not-found/src/main.ts new file mode 100644 index 000000000..e69de29bb