Skip to content

Commit

Permalink
Ignore __esModule annotation when "type": "module" is set
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarred-Sumner committed Jun 25, 2023
1 parent 501c025 commit bbb2e38
Show file tree
Hide file tree
Showing 17 changed files with 103 additions and 36 deletions.
6 changes: 5 additions & 1 deletion src/bun.js/bindings/CommonJSModuleRecord.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ void JSCommonJSModule::toSyntheticSource(JSC::JSGlobalObject* globalObject,
auto catchScope = DECLARE_CATCH_SCOPE(vm);

Identifier esModuleMarker = builtinNames(vm).__esModulePublicName();
bool hasESModuleMarker = exports->hasProperty(globalObject, esModuleMarker);
bool hasESModuleMarker = !this->ignoreESModuleAnnotation && exports->hasProperty(globalObject, esModuleMarker);
if (catchScope.exception()) {
catchScope.clearException();
}
Expand Down Expand Up @@ -818,6 +818,7 @@ bool JSCommonJSModule::evaluate(
{
auto& vm = globalObject->vm();
auto sourceProvider = Zig::SourceProvider::create(jsCast<Zig::GlobalObject*>(globalObject), source, JSC::SourceProviderSourceType::Program);
this->ignoreESModuleAnnotation = source.tag == ResolvedSourceTagPackageJSONTypeModule;
JSC::SourceCode rawInputSource(
WTFMove(sourceProvider));

Expand Down Expand Up @@ -856,6 +857,7 @@ std::optional<JSC::SourceCode> createCommonJSModule(
JSValue entry = globalObject->requireMap()->get(globalObject, specifierValue);

auto sourceProvider = Zig::SourceProvider::create(jsCast<Zig::GlobalObject*>(globalObject), source, JSC::SourceProviderSourceType::Program);
bool ignoreESModuleAnnotation = source.tag == ResolvedSourceTagPackageJSONTypeModule;
SourceOrigin sourceOrigin = sourceProvider->sourceOrigin();

if (entry) {
Expand Down Expand Up @@ -887,6 +889,8 @@ std::optional<JSC::SourceCode> createCommonJSModule(
globalObject->requireMap()->set(globalObject, requireMapKey, moduleObject);
}

moduleObject->ignoreESModuleAnnotation = ignoreESModuleAnnotation;

return JSC::SourceCode(
JSC::SyntheticSourceProvider::create(
[](JSC::JSGlobalObject* lexicalGlobalObject,
Expand Down
1 change: 1 addition & 0 deletions src/bun.js/bindings/CommonJSModuleRecord.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class JSCommonJSModule final : public JSC::JSDestructibleObject {
mutable JSC::WriteBarrier<JSC::JSString> m_filename;
mutable JSC::WriteBarrier<JSC::JSString> m_dirname;
mutable JSC::WriteBarrier<JSC::JSSourceCode> sourceCode;
bool ignoreESModuleAnnotation { false };

static void destroy(JSC::JSCell*);
~JSCommonJSModule();
Expand Down
7 changes: 4 additions & 3 deletions src/bun.js/bindings/exports.zig
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,10 @@ pub const ResolvedSource = extern struct {

pub const Tag = enum(u64) {
javascript = 0,
wasm = 1,
object = 2,
file = 3,
package_json_type_module = 1,
wasm = 2,
object = 3,
file = 4,

@"node:buffer" = 1024,
@"node:process" = 1025,
Expand Down
1 change: 1 addition & 0 deletions src/bun.js/bindings/headers-handwritten.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ typedef struct ResolvedSource {
void* allocator;
uint64_t tag;
} ResolvedSource;
static const uint64_t ResolvedSourceTagPackageJSONTypeModule = 1;
typedef union ErrorableResolvedSourceResult {
ResolvedSource value;
ZigErrorType err;
Expand Down
21 changes: 21 additions & 0 deletions src/bun.js/module_loader.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,25 @@ pub const ModuleLoader = struct {
return resolved_source;
}

// Pass along package.json type "module" if set.
const tag = brk: {
if (parse_result.ast.exports_kind == .cjs and parse_result.source.path.isFile()) {
var actual_package_json: *PackageJSON = package_json orelse brk2: {
// this should already be cached virtually always so it's fine to do this
var dir_info = (jsc_vm.bundler.resolver.readDirInfo(parse_result.source.path.name.dir) catch null) orelse
break :brk .javascript;

break :brk2 dir_info.package_json orelse dir_info.enclosing_package_json;
} orelse break :brk .javascript;

if (actual_package_json.module_type == .esm) {
break :brk ResolvedSource.Tag.package_json_type_module;
}
}

break :brk ResolvedSource.Tag.javascript;
};

return .{
.allocator = null,
.source_code = bun.String.createLatin1(printer.ctx.getWritten()),
Expand All @@ -1245,6 +1264,8 @@ pub const ModuleLoader = struct {

// having JSC own the memory causes crashes
.hash = 0,

.tag = tag,
};
},
// provideFetch() should be called
Expand Down
89 changes: 57 additions & 32 deletions test/js/bun/resolve/esModule-annotation.test.js
Original file line number Diff line number Diff line change
@@ -1,43 +1,68 @@
import { test, expect } from "bun:test";
import * as ExportEsModuleAnnotationMissingDefault from "./export-esModule-annotation-empty.cjs";
import * as ExportEsModuleAnnotationNoDefault from "./export-esModule-annotation-no-default.cjs";
import * as ExportEsModuleAnnotation from "./export-esModule-annotation.cjs";
import * as ExportEsModuleNoAnnotation from "./export-esModule-no-annotation.cjs";
import DefaultExportForExportEsModuleAnnotationNoDefault from "./export-esModule-annotation-no-default.cjs";
import DefaultExportForExportEsModuleAnnotation from "./export-esModule-annotation.cjs";
import DefaultExportForExportEsModuleNoAnnotation from "./export-esModule-no-annotation.cjs";

test("empty exports object", () => {
expect(ExportEsModuleAnnotationMissingDefault.default).toBe(undefined);
expect(ExportEsModuleAnnotationMissingDefault.__esModule).toBeUndefined();
});
import { test, expect, describe } from "bun:test";
import * as WithTypeModuleExportEsModuleAnnotationMissingDefault from "./with-type-module/export-esModule-annotation-empty.cjs";
import * as WithTypeModuleExportEsModuleAnnotationNoDefault from "./with-type-module/export-esModule-annotation-no-default.cjs";
import * as WithTypeModuleExportEsModuleAnnotation from "./with-type-module/export-esModule-annotation.cjs";
import * as WithTypeModuleExportEsModuleNoAnnotation from "./with-type-module/export-esModule-no-annotation.cjs";
import * as WithoutTypeModuleExportEsModuleAnnotationMissingDefault from "./without-type-module/export-esModule-annotation-empty.cjs";
import * as WithoutTypeModuleExportEsModuleAnnotationNoDefault from "./without-type-module/export-esModule-annotation-no-default.cjs";
import * as WithoutTypeModuleExportEsModuleAnnotation from "./without-type-module/export-esModule-annotation.cjs";
import * as WithoutTypeModuleExportEsModuleNoAnnotation from "./without-type-module/export-esModule-no-annotation.cjs";

test("exports.__esModule = true", () => {
expect(ExportEsModuleAnnotationNoDefault.default).toEqual({
// in this case, since it's the CommonJS module.exports object, it leaks the __esModule
__esModule: true,
describe('without type: "module"', () => {
test("module.exports = {}", () => {
expect(WithoutTypeModuleExportEsModuleAnnotationMissingDefault.default).toEqual({});
expect(WithoutTypeModuleExportEsModuleAnnotationMissingDefault.__esModule).toBeUndefined();
});

// The module namespace object will not have the __esModule property.
expect(ExportEsModuleAnnotationNoDefault).not.toHaveProperty("__esModule");
test("exports.__esModule = true", () => {
expect(WithoutTypeModuleExportEsModuleAnnotationNoDefault.default).toEqual({
__esModule: true,
});

expect(DefaultExportForExportEsModuleAnnotationNoDefault).toEqual({
__esModule: true,
// The module namespace object will not have the __esModule property.
expect(WithoutTypeModuleExportEsModuleAnnotationNoDefault).not.toHaveProperty("__esModule");
});

test("exports.default = true; exports.__esModule = true;", () => {
expect(WithoutTypeModuleExportEsModuleAnnotation.default).toBeTrue();
expect(WithoutTypeModuleExportEsModuleAnnotation.__esModule).toBeUndefined();
});
});

test("exports.default = true; exports.__esModule = true;", () => {
expect(ExportEsModuleAnnotation.default).toBeTrue();
expect(ExportEsModuleAnnotation.__esModule).toBeUndefined();
expect(DefaultExportForExportEsModuleAnnotation).toBeTrue();
test("exports.default = true;", () => {
expect(WithoutTypeModuleExportEsModuleNoAnnotation.default).toEqual({
default: true,
});
expect(WithoutTypeModuleExportEsModuleAnnotation.__esModule).toBeUndefined();
});
});

test("exports.default = true;", () => {
expect(ExportEsModuleNoAnnotation.default).toEqual({
default: true,
describe('with type: "module"', () => {
test("module.exports = {}", () => {
expect(WithTypeModuleExportEsModuleAnnotationMissingDefault.default).toEqual({});
expect(WithTypeModuleExportEsModuleAnnotationMissingDefault.__esModule).toBeUndefined();
});
expect(ExportEsModuleAnnotation.__esModule).toBeUndefined();
expect(DefaultExportForExportEsModuleNoAnnotation).toEqual({
default: true,

test("exports.__esModule = true", () => {
expect(WithTypeModuleExportEsModuleAnnotationNoDefault.default).toEqual({
__esModule: true,
});

// The module namespace object WILL have the __esModule property.
expect(WithTypeModuleExportEsModuleAnnotationNoDefault).toHaveProperty("__esModule");
});

test("exports.default = true; exports.__esModule = true;", () => {
expect(WithTypeModuleExportEsModuleAnnotation.default).toEqual({
default: true,
__esModule: true,
});
expect(WithTypeModuleExportEsModuleAnnotation.__esModule).toBeTrue();
});

test("exports.default = true;", () => {
expect(WithTypeModuleExportEsModuleNoAnnotation.default).toEqual({
default: true,
});
expect(WithTypeModuleExportEsModuleAnnotation.__esModule).toBeTrue();
});
});
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = {};
4 changes: 4 additions & 0 deletions test/js/bun/resolve/with-type-module/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "with-type-module",
"type": "module"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = {};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
exports.__esModule = true;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
exports.default = true;
exports.__esModule = true;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
exports.default = true;
4 changes: 4 additions & 0 deletions test/js/bun/resolve/without-type-module/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "without-type-module",
"type": "commonjs"
}

0 comments on commit bbb2e38

Please sign in to comment.