Skip to content

Commit

Permalink
fix #2119: bug with paths-without-baseUrl warning
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Mar 27, 2022
1 parent 747e51e commit 2244c09
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 16 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* Fix edge case regarding `baseUrl` and `paths` in `tsconfig.json` ([#2119](https://github.com/evanw/esbuild/issues/2119))

In `tsconfig.json`, TypeScript forbids non-relative values inside `paths` if `baseUrl` is not present, and esbuild does too. However, TypeScript checked this after the entire `tsconfig.json` hierarchy was parsed while esbuild incorrectly checked this immediately when parsing the file containing the `paths` map. This caused incorrect warnings to be generated for `tsconfig.json` files that specify a `baseUrl` value and that inherit a `paths` value from an `extends` clause. Now esbuild will only check for non-relative `paths` values after the entire hierarchy has been parsed to avoid generating incorrect warnings.

* Better handle errors where the esbuild binary executable is corrupted or missing ([#2129](https://github.com/evanw/esbuild/issues/2129))

If the esbuild binary executable is corrupted or missing, previously there was one situation where esbuild's JavaScript API could hang instead of generating an error. This release changes esbuild's library code to generate an error instead in this case.
Expand Down
97 changes: 97 additions & 0 deletions internal/bundler/bundler_tsconfig_test.go
Expand Up @@ -1417,3 +1417,100 @@ func TestTsconfigOverriddenTargetWarning(t *testing.T) {
`,
})
}

func TestTsConfigNoBaseURLExtendsPaths(t *testing.T) {
tsconfig_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.ts": `
import { foo } from "foo"
console.log(foo)
`,
"/Users/user/project/lib/foo.ts": `
export let foo = 123
`,
"/Users/user/project/tsconfig.json": `{
"extends": "./base/defaults"
}`,
"/Users/user/project/base/defaults.json": `{
"compilerOptions": {
"paths": {
"*": ["lib/*"]
}
}
}`,
},
entryPaths: []string{"/Users/user/project/src/entry.ts"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/Users/user/project/out.js",
},
expectedScanLog: `Users/user/project/base/defaults.json: WARNING: Non-relative path "lib/*" is not allowed when "baseUrl" is not set (did you forget a leading "./"?)
Users/user/project/src/entry.ts: ERROR: Could not resolve "foo"
NOTE: You can mark the path "foo" as external to exclude it from the bundle, which will remove this error.
`,
})
}

func TestTsConfigBaseURLExtendsPaths(t *testing.T) {
tsconfig_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.ts": `
import { foo } from "foo"
console.log(foo)
`,
"/Users/user/project/lib/foo.ts": `
export let foo = 123
`,
"/Users/user/project/tsconfig.json": `{
"extends": "./base/defaults",
"compilerOptions": {
"baseUrl": "."
}
}`,
"/Users/user/project/base/defaults.json": `{
"compilerOptions": {
"paths": {
"*": ["lib/*"]
}
}
}`,
},
entryPaths: []string{"/Users/user/project/src/entry.ts"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/Users/user/project/out.js",
},
})
}

func TestTsConfigPathsExtendsBaseURL(t *testing.T) {
tsconfig_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.ts": `
import { foo } from "foo"
console.log(foo)
`,
"/Users/user/project/base/test/lib/foo.ts": `
export let foo = 123
`,
"/Users/user/project/tsconfig.json": `{
"extends": "./base/defaults",
"compilerOptions": {
"paths": {
"*": ["lib/*"]
}
}
}`,
"/Users/user/project/base/defaults.json": `{
"compilerOptions": {
"baseUrl": "test"
}
}`,
},
entryPaths: []string{"/Users/user/project/src/entry.ts"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/Users/user/project/out.js",
},
})
}
18 changes: 18 additions & 0 deletions internal/bundler/snapshots/snapshots_tsconfig.txt
Expand Up @@ -13,6 +13,15 @@ var require_util = __commonJS({
var import_util = __toESM(require_util());
console.log((0, import_util.default)());

================================================================================
TestTsConfigBaseURLExtendsPaths
---------- /Users/user/project/out.js ----------
// Users/user/project/lib/foo.ts
var foo = 123;

// Users/user/project/src/entry.ts
console.log(foo);

================================================================================
TestTsConfigJSX
---------- /Users/user/project/out.js ----------
Expand Down Expand Up @@ -110,6 +119,15 @@ var baseurl_nested_default = {
// Users/user/project/entry.ts
console.log(baseurl_dot_default, baseurl_nested_default);

================================================================================
TestTsConfigPathsExtendsBaseURL
---------- /Users/user/project/out.js ----------
// Users/user/project/base/test/lib/foo.ts
var foo = 123;

// Users/user/project/src/entry.ts
console.log(foo);

================================================================================
TestTsConfigPathsNoBaseURL
---------- /Users/user/project/out.js ----------
Expand Down
40 changes: 31 additions & 9 deletions internal/resolver/resolver.go
Expand Up @@ -860,6 +860,7 @@ func (r resolverQuery) parseTSConfig(file string, visited map[string]bool) (*TSC
if visited[file] {
return nil, errParseErrorImportCycle
}
isExtends := len(visited) != 0
visited[file] = true

contents, err, originalError := r.caches.FSCache.ReadFile(r.fs, file)
Expand Down Expand Up @@ -965,6 +966,27 @@ func (r resolverQuery) parseTSConfig(file string, visited map[string]bool) (*TSC
result.BaseURLForPaths = r.fs.Join(fileDir, result.BaseURLForPaths)
}

// Now that we have parsed the entire "tsconfig.json" file, filter out any
// paths that are invalid due to being a package-style path without a base
// URL specified. This must be done here instead of when we're parsing the
// original file because TypeScript allows one "tsconfig.json" file to
// specify "baseUrl" and inherit a "paths" from another file via "extends".
if !isExtends && result.Paths != nil && result.BaseURL == nil {
var tracker *logger.LineColumnTracker
for key, paths := range result.Paths.Map {
end := 0
for _, path := range paths {
if isValidTSConfigPathNoBaseURLPattern(path.Text, r.log, &result.Paths.Source, &tracker, path.Loc) {
paths[end] = path
end++
}
}
if end < len(paths) {
result.Paths.Map[key] = paths[:end]
}
}
}

return result, nil
}

Expand Down Expand Up @@ -1486,24 +1508,24 @@ func (r resolverQuery) matchTSConfigPaths(tsConfigJSON *TSConfigJSON, path strin
}

// Check for exact matches first
for key, originalPaths := range tsConfigJSON.Paths {
for key, originalPaths := range tsConfigJSON.Paths.Map {
if key == path {
if r.debugLogs != nil {
r.debugLogs.addNote(fmt.Sprintf("Found an exact match for %q in \"paths\"", key))
}
for _, originalPath := range originalPaths {
// Ignore ".d.ts" files because this rule is obviously only here for type checking
if hasCaseInsensitiveSuffix(originalPath, ".d.ts") {
if hasCaseInsensitiveSuffix(originalPath.Text, ".d.ts") {
if r.debugLogs != nil {
r.debugLogs.addNote(fmt.Sprintf("Ignoring substitution %q because it ends in \".d.ts\"", originalPath))
r.debugLogs.addNote(fmt.Sprintf("Ignoring substitution %q because it ends in \".d.ts\"", originalPath.Text))
}
continue
}

// Load the original path relative to the "baseUrl" from tsconfig.json
absoluteOriginalPath := originalPath
if !r.fs.IsAbs(originalPath) {
absoluteOriginalPath = r.fs.Join(absBaseURL, originalPath)
absoluteOriginalPath := originalPath.Text
if !r.fs.IsAbs(absoluteOriginalPath) {
absoluteOriginalPath = r.fs.Join(absBaseURL, absoluteOriginalPath)
}
if absolute, ok, diffCase := r.loadAsFileOrDirectory(absoluteOriginalPath); ok {
return absolute, true, diffCase
Expand All @@ -1516,14 +1538,14 @@ func (r resolverQuery) matchTSConfigPaths(tsConfigJSON *TSConfigJSON, path strin
type match struct {
prefix string
suffix string
originalPaths []string
originalPaths []TSConfigPath
}

// Check for pattern matches next
longestMatchPrefixLength := -1
longestMatchSuffixLength := -1
var longestMatch match
for key, originalPaths := range tsConfigJSON.Paths {
for key, originalPaths := range tsConfigJSON.Paths.Map {
if starIndex := strings.IndexByte(key, '*'); starIndex != -1 {
prefix, suffix := key[:starIndex], key[starIndex+1:]

Expand Down Expand Up @@ -1555,7 +1577,7 @@ func (r resolverQuery) matchTSConfigPaths(tsConfigJSON *TSConfigJSON, path strin
for _, originalPath := range longestMatch.originalPaths {
// Swap out the "*" in the original path for whatever the "*" matched
matchedText := path[len(longestMatch.prefix) : len(path)-len(longestMatch.suffix)]
originalPath = strings.Replace(originalPath, "*", matchedText, 1)
originalPath := strings.Replace(originalPath.Text, "*", matchedText, 1)

// Ignore ".d.ts" files because this rule is obviously only here for type checking
if hasCaseInsensitiveSuffix(originalPath, ".d.ts") {
Expand Down
30 changes: 23 additions & 7 deletions internal/resolver/tsconfig_json.go
Expand Up @@ -33,7 +33,7 @@ type TSConfigJSON struct {
// the wildcard is substituted into the fallback path. The keys represent
// module-style path names and the fallback paths are relative to the
// "baseUrl" value in the "tsconfig.json" file.
Paths map[string][]string
Paths *TSConfigPaths

TSTarget *config.TSTarget
JSXFactory []string
Expand All @@ -43,6 +43,19 @@ type TSConfigJSON struct {
PreserveValueImports bool
}

type TSConfigPath struct {
Text string
Loc logger.Loc
}

type TSConfigPaths struct {
Map map[string][]TSConfigPath

// This may be different from the original "tsconfig.json" source if the
// "paths" value is from another file via an "extends" clause.
Source logger.Source
}

func ParseTSConfigJSON(
log logger.Log,
source logger.Source,
Expand Down Expand Up @@ -193,7 +206,7 @@ func ParseTSConfigJSON(
} else {
result.BaseURLForPaths = "."
}
result.Paths = make(map[string][]string)
result.Paths = &TSConfigPaths{Source: source, Map: make(map[string][]TSConfigPath)}
for _, prop := range paths.Properties {
if key, ok := getString(prop.Key); ok {
if !isValidTSConfigPathPattern(key, log, &source, &tracker, prop.Key.Loc) {
Expand Down Expand Up @@ -224,9 +237,8 @@ func ParseTSConfigJSON(
if array, ok := prop.ValueOrNil.Data.(*js_ast.EArray); ok {
for _, item := range array.Items {
if str, ok := getString(item); ok {
if isValidTSConfigPathPattern(str, log, &source, &tracker, item.Loc) &&
(hasBaseURL || isValidTSConfigPathNoBaseURLPattern(str, log, &source, &tracker, item.Loc)) {
result.Paths[key] = append(result.Paths[key], str)
if isValidTSConfigPathPattern(str, log, &source, &tracker, item.Loc) {
result.Paths.Map[key] = append(result.Paths.Map[key], TSConfigPath{Text: str, Loc: item.Loc})
}
}
}
Expand Down Expand Up @@ -278,7 +290,7 @@ func isSlash(c byte) bool {
return c == '/' || c == '\\'
}

func isValidTSConfigPathNoBaseURLPattern(text string, log logger.Log, source *logger.Source, tracker *logger.LineColumnTracker, loc logger.Loc) bool {
func isValidTSConfigPathNoBaseURLPattern(text string, log logger.Log, source *logger.Source, tracker **logger.LineColumnTracker, loc logger.Loc) bool {
var c0 byte
var c1 byte
var c2 byte
Expand Down Expand Up @@ -315,7 +327,11 @@ func isValidTSConfigPathNoBaseURLPattern(text string, log logger.Log, source *lo
}

r := source.RangeOfString(loc)
log.Add(logger.Warning, tracker, r, fmt.Sprintf(
if *tracker == nil {
t := logger.MakeLineColumnTracker(source)
*tracker = &t
}
log.Add(logger.Warning, *tracker, r, fmt.Sprintf(
"Non-relative path %q is not allowed when \"baseUrl\" is not set (did you forget a leading \"./\"?)", text))
return false
}

0 comments on commit 2244c09

Please sign in to comment.