Skip to content

Commit

Permalink
fix(cli): treat package.json imports as optional dependencies (#7044)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jbedard committed Oct 23, 2024
1 parent c5f1019 commit 62685ba
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 2 deletions.
5 changes: 4 additions & 1 deletion gazelle/js/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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,
})
}
}
Expand Down
4 changes: 3 additions & 1 deletion gazelle/js/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
3 changes: 3 additions & 0 deletions gazelle/js/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
},
"./bin": "./bin/does-not-exist.js",
"./dne": "./does-not-exist.js",
"./not-ignored-dne": "./not-ignored.js",
"./lib": "./lib1.js"
}
}
1 change: 1 addition & 0 deletions gazelle/js/tests/npm_package_deps_lib/not-found/BUILD.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# gazelle:generation_mode update
8 changes: 8 additions & 0 deletions gazelle/js/tests/npm_package_deps_lib/not-found/BUILD.out
Original file line number Diff line number Diff line change
@@ -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"],
)
10 changes: 10 additions & 0 deletions gazelle/js/tests/npm_package_deps_lib/not-found/package.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
Empty file.

0 comments on commit 62685ba

Please sign in to comment.