Skip to content

Commit

Permalink
fix #446: bug with default+namespace imports
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Oct 10, 2020
1 parent 753eb22 commit 4356e31
Show file tree
Hide file tree
Showing 5 changed files with 316 additions and 38 deletions.
26 changes: 26 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,31 @@
# Changelog

## Unreleased

* Fix a bug with compound import statements ([#446](https://github.com/evanw/esbuild/issues/446))

Import statements can simultaneously contain both a default import and a namespace import like this:

```js
import defVal, * as nsVal from 'path'
```

These statements were previously miscompiled when bundling if the import path was marked as external, or when converting to a specific output format, and the namespace variable itself was used for something other than a property access. The generated code contained a syntax error because it generated a `{...}` import clause containing the default import.

This particular problem was caused by code that converts namespace imports into import clauses for more efficient bundling. This transformation should not be done if the namespace import cannot be completely removed:

```js
// Can convert namespace to clause
import defVal, * as nsVal from 'path'
console.log(defVal, nsVal.prop)
```

```js
// Cannot convert namespace to clause
import defVal, * as nsVal from 'path'
console.log(defVal, nsVal)
```

## 0.7.13

* Fix `mainFields` in the JavaScript API ([#440](https://github.com/evanw/esbuild/issues/440) and [#441](https://github.com/evanw/esbuild/pull/441))
Expand Down
121 changes: 121 additions & 0 deletions internal/bundler/bundler_importstar_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1423,3 +1423,124 @@ func TestReExportStarAsCommonJSNoBundle(t *testing.T) {
},
})
}

func TestImportDefaultNamespaceComboIssue446(t *testing.T) {
importstar_suite.expectBundled(t, bundled{
files: map[string]string{
"/external-default2.js": `
import def, {default as default2} from 'external'
console.log(def, default2)
`,
"/external-ns.js": `
import def, * as ns from 'external'
console.log(def, ns)
`,
"/external-ns-default.js": `
import def, * as ns from 'external'
console.log(def, ns, ns.default)
`,
"/external-ns-def.js": `
import def, * as ns from 'external'
console.log(def, ns, ns.def)
`,
"/external-default.js": `
import def, * as ns from 'external'
console.log(def, ns.default)
`,
"/external-def.js": `
import def, * as ns from 'external'
console.log(def, ns.def)
`,
"/internal-default2.js": `
import def, {default as default2} from './internal'
console.log(def, default2)
`,
"/internal-ns.js": `
import def, * as ns from './internal'
console.log(def, ns)
`,
"/internal-ns-default.js": `
import def, * as ns from './internal'
console.log(def, ns, ns.default)
`,
"/internal-ns-def.js": `
import def, * as ns from './internal'
console.log(def, ns, ns.def)
`,
"/internal-default.js": `
import def, * as ns from './internal'
console.log(def, ns.default)
`,
"/internal-def.js": `
import def, * as ns from './internal'
console.log(def, ns.def)
`,
"/internal.js": `
export default 123
`,
},
entryPaths: []string{
"/external-default2.js",
"/external-ns.js",
"/external-ns-default.js",
"/external-ns-def.js",
"/external-default.js",
"/external-def.js",
"/internal-default2.js",
"/internal-ns.js",
"/internal-ns-default.js",
"/internal-ns-def.js",
"/internal-default.js",
"/internal-def.js",
},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
"external": true,
},
},
},
expectedCompileLog: `/internal-def.js: warning: No matching export for import "def"
/internal-ns-def.js: warning: No matching export for import "def"
`,
})
}

func TestImportDefaultNamespaceComboNoDefault(t *testing.T) {
importstar_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry-default-ns-prop.js": `import def, * as ns from './foo'; console.log(def, ns, ns.default)`,
"/entry-default-ns.js": `import def, * as ns from './foo'; console.log(def, ns)`,
"/entry-default-prop.js": `import def, * as ns from './foo'; console.log(def, ns.default)`,
"/entry-default.js": `import def from './foo'; console.log(def)`,
"/entry-prop.js": `import * as ns from './foo'; console.log(ns.default)`,
"/foo.js": `export let foo = 123`,
},
entryPaths: []string{
"/entry-default-ns-prop.js",
"/entry-default-ns.js",
"/entry-default-prop.js",
"/entry-default.js",
"/entry-prop.js",
},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
ExternalModules: config.ExternalModules{
NodeModules: map[string]bool{
"external": true,
},
},
},
expectedCompileLog: `/entry-default-ns-prop.js: error: No matching export for import "default"
/entry-default-ns-prop.js: warning: No matching export for import "default"
/entry-default-ns.js: error: No matching export for import "default"
/entry-default-prop.js: error: No matching export for import "default"
/entry-default-prop.js: warning: No matching export for import "default"
/entry-default.js: error: No matching export for import "default"
/entry-prop.js: warning: No matching export for import "default"
`,
})
}
94 changes: 92 additions & 2 deletions internal/bundler/snapshots/snapshots_importstar.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,96 @@ __export(exports, {
// /foo.js
let foo = "foo";

================================================================================
TestImportDefaultNamespaceComboIssue446
---------- /out/external-default2.js ----------
// /external-default2.js
import def, {default as default2} from "external";
console.log(def, default2);

---------- /out/external-ns.js ----------
// /external-ns.js
import def, * as ns from "external";
console.log(def, ns);

---------- /out/external-ns-default.js ----------
// /external-ns-default.js
import def, * as ns from "external";
console.log(def, ns, ns.default);

---------- /out/external-ns-def.js ----------
// /external-ns-def.js
import def, * as ns from "external";
console.log(def, ns, ns.def);

---------- /out/external-default.js ----------
// /external-default.js
import def, {
default as default2
} from "external";
console.log(def, default2);

---------- /out/external-def.js ----------
// /external-def.js
import def, {
def as def2
} from "external";
console.log(def, def2);

---------- /out/internal-default2.js ----------
// /internal.js
var internal_default = 123;

// /internal-default2.js
console.log(internal_default, internal_default);

---------- /out/internal-ns.js ----------
// /internal.js
const internal_exports = {};
__export(internal_exports, {
default: () => internal_default
});
var internal_default = 123;

// /internal-ns.js
console.log(internal_default, internal_exports);

---------- /out/internal-ns-default.js ----------
// /internal.js
const internal_exports = {};
__export(internal_exports, {
default: () => internal_default
});
var internal_default = 123;

// /internal-ns-default.js
console.log(internal_default, internal_exports, internal_default);

---------- /out/internal-ns-def.js ----------
// /internal.js
const internal_exports = {};
__export(internal_exports, {
default: () => internal_default
});
var internal_default = 123;

// /internal-ns-def.js
console.log(internal_default, internal_exports, void 0);

---------- /out/internal-default.js ----------
// /internal.js
var internal_default = 123;

// /internal-default.js
console.log(internal_default, internal_default);

---------- /out/internal-def.js ----------
// /internal.js
var internal_default = 123;

// /internal-def.js
console.log(internal_default, void 0);

================================================================================
TestImportExportOtherAsNamespaceCommonJS
---------- /out.js ----------
Expand Down Expand Up @@ -261,8 +351,8 @@ var require_foo = __commonJS((exports) => {

// /entry.js
const ns = __toModule(require_foo());
let foo2 = 234;
console.log(ns, ns.foo, foo2);
let foo = 234;
console.log(ns, ns.foo, foo);

================================================================================
TestImportStarCommonJSNoCapture
Expand Down
4 changes: 2 additions & 2 deletions internal/bundler/snapshots/snapshots_importstar_ts.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ var require_foo = __commonJS((exports) => {

// /entry.ts
const ns = __toModule(require_foo());
let foo2 = 234;
console.log(ns, ns.foo, foo2);
let foo = 234;
console.log(ns, ns.foo, foo);

================================================================================
TestTSImportStarCommonJSNoCapture
Expand Down
Loading

0 comments on commit 4356e31

Please sign in to comment.