Skip to content

Commit

Permalink
fix #3354: tsconfig.json regression with paths
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Oct 17, 2023
1 parent 72b1e8b commit d5f397f
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 29 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* Fix a regression in 0.19.0 regarding `paths` in `tsconfig.json` ([#3354](https://github.com/evanw/esbuild/issues/3354))

The fix in esbuild version 0.19.0 to process `tsconfig.json` aliases before the `--packages=external` setting unintentionally broke an edge case in esbuild's handling of certain `tsconfig.json` aliases where there are multiple files with the same name in different directories. This release adjusts esbuild's behavior for this edge case so that it passes while still processing aliases before `--packages=external`. Please read the linked issue for more details.

* Fix a CSS `font` property minification bug ([#3452](https://github.com/evanw/esbuild/issues/3452))

This release fixes a bug where esbuild's CSS minifier didn't insert a space between the font size and the font family in the `font` CSS shorthand property in the edge case where the original source code didn't already have a space and the leading string token was shortened to an identifier:
Expand Down
43 changes: 43 additions & 0 deletions internal/bundler_tests/bundler_tsconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2538,3 +2538,46 @@ func TestTsconfigJsonBaseUrlIssue3307(t *testing.T) {
},
})
}

// https://github.com/evanw/esbuild/issues/3354
func TestTsconfigJsonAsteriskNameCollisionIssue3354(t *testing.T) {
tsconfig_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.ts": `
import {foo} from "foo";
foo();
`,
"/Users/user/project/src/tsconfig.json": `
{
"compilerOptions": {
"baseUrl": ".",
"paths": {
"*": ["web/*"]
}
}
}
`,
"/Users/user/project/src/web/foo.ts": `
import {foo as barFoo} from 'bar/foo';
export function foo() {
console.log('web/foo');
barFoo();
}
`,
"/Users/user/project/src/web/bar/foo/foo.ts": `
export function foo() {
console.log('bar/foo');
}
`,
"/Users/user/project/src/web/bar/foo/index.ts": `
export {foo} from './foo'
`,
},
entryPaths: []string{"entry.ts"},
absWorkingDir: "/Users/user/project/src",
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/Users/user/project/out.js",
},
})
}
17 changes: 17 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_tsconfig.txt
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,23 @@ var require_util = __commonJS({
var import_util = __toESM(require_util());
console.log((0, import_util.default)());

================================================================================
TestTsconfigJsonAsteriskNameCollisionIssue3354
---------- /Users/user/project/out.js ----------
// web/bar/foo/foo.ts
function foo() {
console.log("bar/foo");
}

// web/foo.ts
function foo2() {
console.log("web/foo");
foo();
}

// entry.ts
foo2();

================================================================================
TestTsconfigJsonBaseUrl
---------- /Users/user/project/out.js ----------
Expand Down
52 changes: 23 additions & 29 deletions internal/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -993,36 +993,12 @@ func (r resolverQuery) resolveWithoutSymlinks(sourceDir string, sourceDirInfo *d
}
}

// First, check path overrides from the nearest enclosing TypeScript "tsconfig.json" file
if sourceDirInfo != nil {
if tsConfigJSON := r.tsConfigForDir(sourceDirInfo); tsConfigJSON != nil && tsConfigJSON.Paths != nil {
if absolute, ok, diffCase := r.matchTSConfigPaths(tsConfigJSON, importPath); ok {
return &ResolveResult{PathPair: absolute, DifferentCase: diffCase}
}
}
}

// Check both relative and package paths for CSS URL tokens, with relative
// paths taking precedence over package paths to match Webpack behavior.
isPackagePath := IsPackagePath(importPath)
checkRelative := !isPackagePath || r.kind.IsFromCSS()
checkPackage := isPackagePath

// "import 'pkg'" when all packages are external (vs. "import './pkg'"). This
// is deliberately done after we check for "tsconfig.json" aliases because
// people want to be able to make things look like packages but have them not
// be packages.
if r.options.ExternalPackages && isPackagePath && !strings.HasPrefix(importPath, "#") {
if r.debugLogs != nil {
r.debugLogs.addNote("Marking this path as external because it's a package path")
}

r.flushDebugLogs(flushDueToSuccess)
return &ResolveResult{
PathPair: PathPair{Primary: logger.Path{Text: importPath}, IsExternal: true},
}
}

if checkRelative {
absPath := r.fs.Join(sourceDir, importPath)

Expand Down Expand Up @@ -2301,11 +2277,21 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb
defer r.debugLogs.decreaseIndent()
}

// Try looking up the path relative to the base URL from "tsconfig.json"
if tsConfigJSON := r.tsConfigForDir(dirInfo); tsConfigJSON != nil && tsConfigJSON.BaseURL != nil {
basePath := r.fs.Join(*tsConfigJSON.BaseURL, importPath)
if absolute, ok, diffCase := r.loadAsFileOrDirectory(basePath); ok {
return absolute, true, diffCase
// First, check path overrides from the nearest enclosing TypeScript "tsconfig.json" file
if tsConfigJSON := r.tsConfigForDir(dirInfo); tsConfigJSON != nil {
// Try path substitutions first
if tsConfigJSON.Paths != nil {
if absolute, ok, diffCase := r.matchTSConfigPaths(tsConfigJSON, importPath); ok {
return absolute, true, diffCase
}
}

// Try looking up the path relative to the base URL
if tsConfigJSON.BaseURL != nil {
basePath := r.fs.Join(*tsConfigJSON.BaseURL, importPath)
if absolute, ok, diffCase := r.loadAsFileOrDirectory(basePath); ok {
return absolute, true, diffCase
}
}
}

Expand All @@ -2320,6 +2306,14 @@ func (r resolverQuery) loadNodeModules(importPath string, dirInfo *dirInfo, forb
return r.loadPackageImports(importPath, dirInfoPackageJSON)
}

// "import 'pkg'" when all packages are external (vs. "import './pkg'")
if r.options.ExternalPackages && IsPackagePath(importPath) {
if r.debugLogs != nil {
r.debugLogs.addNote("Marking this path as external because it's a package path")
}
return PathPair{Primary: logger.Path{Text: importPath}, IsExternal: true}, true, nil
}

// If Yarn PnP is active, use it to find the package
if r.pnpManifest != nil {
if result := r.resolveToUnqualified(importPath, dirInfo.absPath, r.pnpManifest); result.status.isError() {
Expand Down

0 comments on commit d5f397f

Please sign in to comment.