Skip to content

Commit

Permalink
resolve "browser" keys without extensions (#740)
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Feb 3, 2021
1 parent 94704f6 commit 7f91cec
Show file tree
Hide file tree
Showing 4 changed files with 299 additions and 7 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

* Resolve `browser` entries in `package.json` with no file extension ([#740](https://github.com/evanw/esbuild/issues/740))

This fix changes how esbuild interprets the `browser` field in `package.json`. It will now remap imports without a file extension to `browser` map entries without a file extension, which improves compatibility with Webpack. Specifically, a `package.json` file with `"browser": {"./file": "./something.js"}` will now match an import of `./file`. Previously the `package.json` file had to contain something like `"browser": {"./file.js": "./something.js"}` instead. Note that for compatibility with the rest of the ecosystem, a remapping of `./file` will counter-intuitively _not_ match an import of `./file.js` even though it works fine in the other direction.

## 0.8.39

* Fix the JavaScript watch mode API exiting early ([#730](https://github.com/evanw/esbuild/issues/730))
Expand Down
168 changes: 168 additions & 0 deletions internal/bundler/bundler_packagejson_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,174 @@ func TestPackageJsonBrowserWithMainNode(t *testing.T) {
})
}

func TestPackageJsonBrowserNodeModulesNoExt(t *testing.T) {
packagejson_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import {browser as a} from 'demo-pkg/no-ext'
import {node as b} from 'demo-pkg/no-ext.js'
import {browser as c} from 'demo-pkg/ext'
import {browser as d} from 'demo-pkg/ext.js'
console.log(a)
console.log(b)
console.log(c)
console.log(d)
`,
"/Users/user/project/node_modules/demo-pkg/package.json": `
{
"browser": {
"./no-ext": "./no-ext-browser.js",
"./ext.js": "./ext-browser.js"
}
}
`,
"/Users/user/project/node_modules/demo-pkg/no-ext.js": `
export let node = 'node'
`,
"/Users/user/project/node_modules/demo-pkg/no-ext-browser.js": `
export let browser = 'browser'
`,
"/Users/user/project/node_modules/demo-pkg/ext.js": `
export let node = 'node'
`,
"/Users/user/project/node_modules/demo-pkg/ext-browser.js": `
export let browser = 'browser'
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/Users/user/project/out.js",
},
})
}

func TestPackageJsonBrowserNodeModulesIndexNoExt(t *testing.T) {
packagejson_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import {browser as a} from 'demo-pkg/no-ext'
import {node as b} from 'demo-pkg/no-ext/index.js'
import {browser as c} from 'demo-pkg/ext'
import {browser as d} from 'demo-pkg/ext/index.js'
console.log(a)
console.log(b)
console.log(c)
console.log(d)
`,
"/Users/user/project/node_modules/demo-pkg/package.json": `
{
"browser": {
"./no-ext": "./no-ext-browser/index.js",
"./ext/index.js": "./ext-browser/index.js"
}
}
`,
"/Users/user/project/node_modules/demo-pkg/no-ext/index.js": `
export let node = 'node'
`,
"/Users/user/project/node_modules/demo-pkg/no-ext-browser/index.js": `
export let browser = 'browser'
`,
"/Users/user/project/node_modules/demo-pkg/ext/index.js": `
export let node = 'node'
`,
"/Users/user/project/node_modules/demo-pkg/ext-browser/index.js": `
export let browser = 'browser'
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/Users/user/project/out.js",
},
})
}

func TestPackageJsonBrowserNoExt(t *testing.T) {
packagejson_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import {browser as a} from './demo-pkg/no-ext'
import {node as b} from './demo-pkg/no-ext.js'
import {browser as c} from './demo-pkg/ext'
import {browser as d} from './demo-pkg/ext.js'
console.log(a)
console.log(b)
console.log(c)
console.log(d)
`,
"/Users/user/project/src/demo-pkg/package.json": `
{
"browser": {
"./no-ext": "./no-ext-browser.js",
"./ext.js": "./ext-browser.js"
}
}
`,
"/Users/user/project/src/demo-pkg/no-ext.js": `
export let node = 'node'
`,
"/Users/user/project/src/demo-pkg/no-ext-browser.js": `
export let browser = 'browser'
`,
"/Users/user/project/src/demo-pkg/ext.js": `
export let node = 'node'
`,
"/Users/user/project/src/demo-pkg/ext-browser.js": `
export let browser = 'browser'
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/Users/user/project/out.js",
},
})
}

func TestPackageJsonBrowserIndexNoExt(t *testing.T) {
packagejson_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import {browser as a} from './demo-pkg/no-ext'
import {node as b} from './demo-pkg/no-ext/index.js'
import {browser as c} from './demo-pkg/ext'
import {browser as d} from './demo-pkg/ext/index.js'
console.log(a)
console.log(b)
console.log(c)
console.log(d)
`,
"/Users/user/project/src/demo-pkg/package.json": `
{
"browser": {
"./no-ext": "./no-ext-browser/index.js",
"./ext/index.js": "./ext-browser/index.js"
}
}
`,
"/Users/user/project/src/demo-pkg/no-ext/index.js": `
export let node = 'node'
`,
"/Users/user/project/src/demo-pkg/no-ext-browser/index.js": `
export let browser = 'browser'
`,
"/Users/user/project/src/demo-pkg/ext/index.js": `
export let node = 'node'
`,
"/Users/user/project/src/demo-pkg/ext-browser/index.js": `
export let browser = 'browser'
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/Users/user/project/out.js",
},
})
}

func TestPackageJsonDualPackageHazardImportOnly(t *testing.T) {
packagejson_suite.expectBundled(t, bundled{
files: map[string]string{
Expand Down
72 changes: 72 additions & 0 deletions internal/bundler/snapshots/snapshots_packagejson.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,24 @@ var require_demo_pkg = __commonJS((exports, module) => {
var import_demo_pkg = __toModule(require_demo_pkg());
console.log(import_demo_pkg.default());

================================================================================
TestPackageJsonBrowserIndexNoExt
---------- /Users/user/project/out.js ----------
// Users/user/project/src/demo-pkg/no-ext-browser/index.js
var browser = "browser";

// Users/user/project/src/demo-pkg/no-ext/index.js
var node = "node";

// Users/user/project/src/demo-pkg/ext-browser/index.js
var browser2 = "browser";

// Users/user/project/src/entry.js
console.log(browser);
console.log(node);
console.log(browser2);
console.log(browser2);

================================================================================
TestPackageJsonBrowserMapAvoidMissing
---------- /Users/user/project/out.js ----------
Expand Down Expand Up @@ -170,6 +188,60 @@ var require_main_browser = __commonJS((exports, module) => {
var import_demo_pkg = __toModule(require_main_browser());
console.log(import_demo_pkg.default());

================================================================================
TestPackageJsonBrowserNoExt
---------- /Users/user/project/out.js ----------
// Users/user/project/src/demo-pkg/no-ext-browser.js
var browser = "browser";

// Users/user/project/src/demo-pkg/no-ext.js
var node = "node";

// Users/user/project/src/demo-pkg/ext-browser.js
var browser2 = "browser";

// Users/user/project/src/entry.js
console.log(browser);
console.log(node);
console.log(browser2);
console.log(browser2);

================================================================================
TestPackageJsonBrowserNodeModulesIndexNoExt
---------- /Users/user/project/out.js ----------
// Users/user/project/node_modules/demo-pkg/no-ext-browser/index.js
var browser = "browser";

// Users/user/project/node_modules/demo-pkg/no-ext/index.js
var node = "node";

// Users/user/project/node_modules/demo-pkg/ext-browser/index.js
var browser2 = "browser";

// Users/user/project/src/entry.js
console.log(browser);
console.log(node);
console.log(browser2);
console.log(browser2);

================================================================================
TestPackageJsonBrowserNodeModulesNoExt
---------- /Users/user/project/out.js ----------
// Users/user/project/node_modules/demo-pkg/no-ext-browser.js
var browser = "browser";

// Users/user/project/node_modules/demo-pkg/no-ext.js
var node = "node";

// Users/user/project/node_modules/demo-pkg/ext-browser.js
var browser2 = "browser";

// Users/user/project/src/entry.js
console.log(browser);
console.log(node);
console.log(browser2);
console.log(browser2);

================================================================================
TestPackageJsonBrowserOverMainNode
---------- /Users/user/project/out.js ----------
Expand Down
60 changes: 53 additions & 7 deletions internal/resolver/resolver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,11 +391,29 @@ func (r *resolver) resolveWithoutSymlinks(sourceDir string, importPath string, k
return &ResolveResult{PathPair: PathPair{Primary: logger.Path{Text: absPath, Namespace: "file"}}, IsExternal: true}
}

if absolute, ok := r.loadAsFileOrDirectory(absPath, kind); ok {
checkPackage = false
result = absolute
} else if !checkPackage {
return nil
// Check the non-package "browser" map for the first time (1 out of 2)
importDirInfo := r.dirInfoCached(r.fs.Dir(absPath))
if importDirInfo != nil && importDirInfo.enclosingBrowserScope != nil {
if packageJSON := importDirInfo.enclosingBrowserScope.packageJSON; packageJSON.browserNonPackageMap != nil {
if remapped, ok := packageJSON.browserNonPackageMap[absPath]; ok {
if remapped == nil {
return &ResolveResult{PathPair: PathPair{Primary: logger.Path{Text: absPath, Namespace: BrowserFalseNamespace}}}
} else if remappedResult, ok := r.resolveWithoutRemapping(importDirInfo.enclosingBrowserScope, *remapped, kind); ok {
result = remappedResult
checkRelative = false
checkPackage = false
}
}
}
}

if checkRelative {
if absolute, ok := r.loadAsFileOrDirectory(absPath, kind); ok {
checkPackage = false
result = absolute
} else if !checkPackage {
return nil
}
}
}

Expand Down Expand Up @@ -463,7 +481,7 @@ func (r *resolver) resolveWithoutSymlinks(sourceDir string, importPath string, k
resultDir := r.fs.Dir(path.Text)
resultDirInfo := r.dirInfoCached(resultDir)

// Support remapping one non-module path to another via the "browser" field
// Check the non-package "browser" map for the second time (2 out of 2)
if resultDirInfo != nil && resultDirInfo.enclosingBrowserScope != nil {
packageJSON := resultDirInfo.enclosingBrowserScope.packageJSON
if packageJSON.browserNonPackageMap != nil {
Expand Down Expand Up @@ -530,6 +548,18 @@ type packageJSON struct {
// tell, the official spec is a GitHub repo hosted by a user account:
// https://github.com/defunctzombie/package-browser-field-spec. The npm docs
// say almost nothing: https://docs.npmjs.com/files/package.json.
//
// Note that the non-package "browser" map has to be checked twice to match
// Webpack's behavior: once before resolution and once after resolution. It
// leads to some unintuitive failure cases that we must emulate around missing
// file extensions:
//
// * Given the mapping "./no-ext": "./no-ext-browser.js" the query "./no-ext"
// should match but the query "./no-ext.js" should NOT match.
//
// * Given the mapping "./ext.js": "./ext-browser.js" the query "./ext.js"
// should match and the query "./ext" should ALSO match.
//
browserNonPackageMap map[string]*string
browserPackageMap map[string]*string

Expand Down Expand Up @@ -1252,7 +1282,23 @@ func (r *resolver) loadNodeModules(path string, kind ast.ImportKind, dirInfo *di
// 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), kind)
absPath := r.fs.Join(dirInfo.absPath, "node_modules", path)

// Check the non-package "browser" map for the first time (1 out of 2)
importDirInfo := r.dirInfoCached(r.fs.Dir(absPath))
if importDirInfo != nil && importDirInfo.enclosingBrowserScope != nil {
if packageJSON := importDirInfo.enclosingBrowserScope.packageJSON; packageJSON.browserNonPackageMap != nil {
if remapped, ok := packageJSON.browserNonPackageMap[absPath]; ok {
if remapped == nil {
return PathPair{Primary: logger.Path{Text: absPath, Namespace: BrowserFalseNamespace}}, true
} else if remappedResult, ok := r.resolveWithoutRemapping(importDirInfo.enclosingBrowserScope, *remapped, kind); ok {
return remappedResult, true
}
}
}
}

absolute, ok := r.loadAsFileOrDirectory(absPath, kind)
if ok {
return absolute, true
}
Expand Down

0 comments on commit 7f91cec

Please sign in to comment.