Skip to content

Commit

Permalink
fix #278: node_modules and tsconfig.json ordering
Browse files Browse the repository at this point in the history
evanw committed Jul 25, 2020

Partially verified

This commit is signed with the committer’s verified signature.
targos’s contribution has been verified via GPG key.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
1 parent 2241b21 commit 3e027f6
Showing 3 changed files with 131 additions and 22 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -12,6 +12,10 @@

Specifically, the install script now uses the URL from the `npm_config_registry` environment variable if present instead of the default registry URL `https://registry.npmjs.org/`. Note that the URL must have both a protocol and a host name.

* Fixed ordering between `node_modules` and a force-overridden `tsconfig.json` ([#278](https://github.com/evanw/esbuild/issues/278))

When the `tsconfig.json` settings have been force-overridden using the new `--tsconfig` flag, the path resolution behavior behaved subtly differently than if esbuild naturally discovers the `tsconfig.json` file without the flag. The difference caused package paths present in a `node_modules` folder to incorrectly take precedence over custom path aliases configured in `tsconfig.json`. The ordering has been corrected such that custom path aliases always take place over `node_modules`.

## 0.6.5

* Fix IIFE wrapper for ES5
101 changes: 101 additions & 0 deletions internal/bundler/bundler_tsconfig_test.go
Original file line number Diff line number Diff line change
@@ -581,3 +581,104 @@ console.log(/* @__PURE__ */ worked("div", null));
},
})
}

func TestTsconfigJsonOverrideMissing(t *testing.T) {
expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/app/entry.ts": `
import 'foo'
`,
"/Users/user/project/src/foo-bad.ts": `
console.log('bad')
`,
"/Users/user/project/src/tsconfig.json": `
{
"compilerOptions": {
"baseUrl": ".",
"paths": {
"foo": ["./foo-bad.ts"]
}
}
}
`,
"/Users/user/project/other/foo-good.ts": `
console.log('good')
`,
"/Users/user/project/other/config-for-ts.json": `
{
"compilerOptions": {
"baseUrl": ".",
"paths": {
"foo": ["./foo-good.ts"]
}
}
}
`,
},
entryPaths: []string{"/Users/user/project/src/app/entry.ts"},
options: config.Options{
IsBundling: true,
AbsOutputFile: "/Users/user/project/out.js",
TsConfigOverride: "/Users/user/project/other/config-for-ts.json",
},
expected: map[string]string{
"/Users/user/project/out.js": `// /Users/user/project/other/foo-good.ts
console.log("good");
// /Users/user/project/src/app/entry.ts
`,
},
})
}

func TestTsconfigJsonOverrideNodeModules(t *testing.T) {
expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/app/entry.ts": `
import 'foo'
`,
"/Users/user/project/src/node_modules/foo/index.js": `
console.log('default')
`,
"/Users/user/project/src/foo-bad.ts": `
console.log('bad')
`,
"/Users/user/project/src/tsconfig.json": `
{
"compilerOptions": {
"baseUrl": ".",
"paths": {
"foo": ["./foo-bad.ts"]
}
}
}
`,
"/Users/user/project/other/foo-good.ts": `
console.log('good')
`,
"/Users/user/project/other/config-for-ts.json": `
{
"compilerOptions": {
"baseUrl": ".",
"paths": {
"foo": ["./foo-good.ts"]
}
}
}
`,
},
entryPaths: []string{"/Users/user/project/src/app/entry.ts"},
options: config.Options{
IsBundling: true,
AbsOutputFile: "/Users/user/project/out.js",
TsConfigOverride: "/Users/user/project/other/config-for-ts.json",
},
expected: map[string]string{
"/Users/user/project/out.js": `// /Users/user/project/other/foo-good.ts
console.log("good");
// /Users/user/project/src/app/entry.ts
`,
},
})
}
48 changes: 26 additions & 22 deletions internal/resolver/resolver.go
Original file line number Diff line number Diff line change
@@ -104,13 +104,10 @@ func (r *resolver) finalizeResolve(path ast.Path, isExternal bool) *ResolveResul
}

// Copy various fields from the nearest enclosing "tsconfig.json" file if present
for info := dirInfo; info != nil; info = info.parent {
if info.tsConfigJson != nil {
result.JSXFactory = info.tsConfigJson.jsxFactory
result.JSXFragment = info.tsConfigJson.jsxFragmentFactory
result.StrictClassFields = info.tsConfigJson.useDefineForClassFields
break
}
if dirInfo.tsConfigJson != nil {
result.JSXFactory = dirInfo.tsConfigJson.jsxFactory
result.JSXFragment = dirInfo.tsConfigJson.jsxFragmentFactory
result.StrictClassFields = dirInfo.tsConfigJson.useDefineForClassFields
}

if entry := dirInfo.entries[base]; entry.Symlink != "" {
@@ -341,7 +338,7 @@ type dirInfo struct {
hasNodeModules bool // Is there a "node_modules" subdirectory?
absPathIndex *string // Is there an "index.js" file?
packageJson *packageJson // Is there a "package.json" file?
tsConfigJson *tsConfigJson // Is there a "tsconfig.json" file?
tsConfigJson *tsConfigJson // Is there a "tsconfig.json" file in this directory or a parent directory?
absRealPath string // If non-empty, this is the real absolute path resolving any symlinks
}

@@ -639,6 +636,11 @@ func (r *resolver) dirInfoUncached(path string) *dirInfo {
info.tsConfigJson, _ = r.parseJsTsConfig(forceTsConfig, make(map[string]bool))
}

// Propagate the enclosing tsconfig.json from the parent directory
if info.tsConfigJson == nil && parentInfo != nil {
info.tsConfigJson = parentInfo.tsConfigJson
}

// Is the "main" field from "package.json" missing?
if info.packageJson == nil || info.packageJson.absPathMain == nil {
// Look for an "index" file with known extensions
@@ -968,24 +970,26 @@ func (r *resolver) matchTSConfigPaths(tsConfigJson *tsConfigJson, path string) (
}

func (r *resolver) loadNodeModules(path string, dirInfo *dirInfo) (string, bool) {
for {
// Handle TypeScript base URLs for TypeScript code
if dirInfo.tsConfigJson != nil && dirInfo.tsConfigJson.absPathBaseUrl != nil {
// Try path substitutions first
if dirInfo.tsConfigJson.paths != nil {
if absolute, ok := r.matchTSConfigPaths(dirInfo.tsConfigJson, path); ok {
return absolute, true
}
}

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

// Skip "node_modules" folders
// Try looking up the path relative to the base URL
basePath := r.fs.Join(*dirInfo.tsConfigJson.absPathBaseUrl, path)
if absolute, ok := r.loadAsFileOrDirectory(basePath); ok {
return absolute, true
}
}

// Then check for the package in any enclosing "node_modules" directories
for {
// Skip directories that are themselves called "node_modules", since we
// don't ever want to search for "node_modules/node_modules"
if dirInfo.hasNodeModules {
absolute, ok := r.loadAsFileOrDirectory(r.fs.Join(dirInfo.absPath, "node_modules", path))
if ok {

0 comments on commit 3e027f6

Please sign in to comment.