From bbb2e38810067ed8c7449b5f5879b6fc57d9f88c Mon Sep 17 00:00:00 2001 From: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Date: Sun, 25 Jun 2023 06:45:29 -0700 Subject: [PATCH] Ignore `__esModule` annotation when `"type": "module"` is set --- src/bun.js/bindings/CommonJSModuleRecord.cpp | 6 +- src/bun.js/bindings/CommonJSModuleRecord.h | 1 + src/bun.js/bindings/exports.zig | 7 +- src/bun.js/bindings/headers-handwritten.h | 1 + src/bun.js/module_loader.zig | 21 +++++ .../bun/resolve/esModule-annotation.test.js | 89 ++++++++++++------- .../export-esModule-annotation-empty.cjs | 0 .../export-esModule-annotation-empty.cjs | 1 + .../export-esModule-annotation-no-default.cjs | 0 .../export-esModule-annotation.cjs | 0 .../export-esModule-no-annotation.cjs | 0 .../bun/resolve/with-type-module/package.json | 4 + .../export-esModule-annotation-empty.cjs | 1 + .../export-esModule-annotation-no-default.cjs | 1 + .../export-esModule-annotation.cjs | 2 + .../export-esModule-no-annotation.cjs | 1 + .../resolve/without-type-module/package.json | 4 + 17 files changed, 103 insertions(+), 36 deletions(-) delete mode 100644 test/js/bun/resolve/export-esModule-annotation-empty.cjs create mode 100644 test/js/bun/resolve/with-type-module/export-esModule-annotation-empty.cjs rename test/js/bun/resolve/{ => with-type-module}/export-esModule-annotation-no-default.cjs (100%) rename test/js/bun/resolve/{ => with-type-module}/export-esModule-annotation.cjs (100%) rename test/js/bun/resolve/{ => with-type-module}/export-esModule-no-annotation.cjs (100%) create mode 100644 test/js/bun/resolve/with-type-module/package.json create mode 100644 test/js/bun/resolve/without-type-module/export-esModule-annotation-empty.cjs create mode 100644 test/js/bun/resolve/without-type-module/export-esModule-annotation-no-default.cjs create mode 100644 test/js/bun/resolve/without-type-module/export-esModule-annotation.cjs create mode 100644 test/js/bun/resolve/without-type-module/export-esModule-no-annotation.cjs create mode 100644 test/js/bun/resolve/without-type-module/package.json diff --git a/src/bun.js/bindings/CommonJSModuleRecord.cpp b/src/bun.js/bindings/CommonJSModuleRecord.cpp index 9d865065c0bc6..4e1bb2d35b56a 100644 --- a/src/bun.js/bindings/CommonJSModuleRecord.cpp +++ b/src/bun.js/bindings/CommonJSModuleRecord.cpp @@ -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(); } @@ -818,6 +818,7 @@ bool JSCommonJSModule::evaluate( { auto& vm = globalObject->vm(); auto sourceProvider = Zig::SourceProvider::create(jsCast(globalObject), source, JSC::SourceProviderSourceType::Program); + this->ignoreESModuleAnnotation = source.tag == ResolvedSourceTagPackageJSONTypeModule; JSC::SourceCode rawInputSource( WTFMove(sourceProvider)); @@ -856,6 +857,7 @@ std::optional createCommonJSModule( JSValue entry = globalObject->requireMap()->get(globalObject, specifierValue); auto sourceProvider = Zig::SourceProvider::create(jsCast(globalObject), source, JSC::SourceProviderSourceType::Program); + bool ignoreESModuleAnnotation = source.tag == ResolvedSourceTagPackageJSONTypeModule; SourceOrigin sourceOrigin = sourceProvider->sourceOrigin(); if (entry) { @@ -887,6 +889,8 @@ std::optional createCommonJSModule( globalObject->requireMap()->set(globalObject, requireMapKey, moduleObject); } + moduleObject->ignoreESModuleAnnotation = ignoreESModuleAnnotation; + return JSC::SourceCode( JSC::SyntheticSourceProvider::create( [](JSC::JSGlobalObject* lexicalGlobalObject, diff --git a/src/bun.js/bindings/CommonJSModuleRecord.h b/src/bun.js/bindings/CommonJSModuleRecord.h index 48f14b39cadc0..a0fd1533b7998 100644 --- a/src/bun.js/bindings/CommonJSModuleRecord.h +++ b/src/bun.js/bindings/CommonJSModuleRecord.h @@ -25,6 +25,7 @@ class JSCommonJSModule final : public JSC::JSDestructibleObject { mutable JSC::WriteBarrier m_filename; mutable JSC::WriteBarrier m_dirname; mutable JSC::WriteBarrier sourceCode; + bool ignoreESModuleAnnotation { false }; static void destroy(JSC::JSCell*); ~JSCommonJSModule(); diff --git a/src/bun.js/bindings/exports.zig b/src/bun.js/bindings/exports.zig index f77b572162a37..213291e7b0f96 100644 --- a/src/bun.js/bindings/exports.zig +++ b/src/bun.js/bindings/exports.zig @@ -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, diff --git a/src/bun.js/bindings/headers-handwritten.h b/src/bun.js/bindings/headers-handwritten.h index 57940550f38a6..db1e38d3e439e 100644 --- a/src/bun.js/bindings/headers-handwritten.h +++ b/src/bun.js/bindings/headers-handwritten.h @@ -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; diff --git a/src/bun.js/module_loader.zig b/src/bun.js/module_loader.zig index 5838d8a4928c1..b25bb4b10502f 100644 --- a/src/bun.js/module_loader.zig +++ b/src/bun.js/module_loader.zig @@ -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()), @@ -1245,6 +1264,8 @@ pub const ModuleLoader = struct { // having JSC own the memory causes crashes .hash = 0, + + .tag = tag, }; }, // provideFetch() should be called diff --git a/test/js/bun/resolve/esModule-annotation.test.js b/test/js/bun/resolve/esModule-annotation.test.js index 6e221fedee17b..33c84be5d52a5 100644 --- a/test/js/bun/resolve/esModule-annotation.test.js +++ b/test/js/bun/resolve/esModule-annotation.test.js @@ -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(); }); }); diff --git a/test/js/bun/resolve/export-esModule-annotation-empty.cjs b/test/js/bun/resolve/export-esModule-annotation-empty.cjs deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/test/js/bun/resolve/with-type-module/export-esModule-annotation-empty.cjs b/test/js/bun/resolve/with-type-module/export-esModule-annotation-empty.cjs new file mode 100644 index 0000000000000..f053ebf7976e3 --- /dev/null +++ b/test/js/bun/resolve/with-type-module/export-esModule-annotation-empty.cjs @@ -0,0 +1 @@ +module.exports = {}; diff --git a/test/js/bun/resolve/export-esModule-annotation-no-default.cjs b/test/js/bun/resolve/with-type-module/export-esModule-annotation-no-default.cjs similarity index 100% rename from test/js/bun/resolve/export-esModule-annotation-no-default.cjs rename to test/js/bun/resolve/with-type-module/export-esModule-annotation-no-default.cjs diff --git a/test/js/bun/resolve/export-esModule-annotation.cjs b/test/js/bun/resolve/with-type-module/export-esModule-annotation.cjs similarity index 100% rename from test/js/bun/resolve/export-esModule-annotation.cjs rename to test/js/bun/resolve/with-type-module/export-esModule-annotation.cjs diff --git a/test/js/bun/resolve/export-esModule-no-annotation.cjs b/test/js/bun/resolve/with-type-module/export-esModule-no-annotation.cjs similarity index 100% rename from test/js/bun/resolve/export-esModule-no-annotation.cjs rename to test/js/bun/resolve/with-type-module/export-esModule-no-annotation.cjs diff --git a/test/js/bun/resolve/with-type-module/package.json b/test/js/bun/resolve/with-type-module/package.json new file mode 100644 index 0000000000000..f1863a426ff2e --- /dev/null +++ b/test/js/bun/resolve/with-type-module/package.json @@ -0,0 +1,4 @@ +{ + "name": "with-type-module", + "type": "module" +} diff --git a/test/js/bun/resolve/without-type-module/export-esModule-annotation-empty.cjs b/test/js/bun/resolve/without-type-module/export-esModule-annotation-empty.cjs new file mode 100644 index 0000000000000..f053ebf7976e3 --- /dev/null +++ b/test/js/bun/resolve/without-type-module/export-esModule-annotation-empty.cjs @@ -0,0 +1 @@ +module.exports = {}; diff --git a/test/js/bun/resolve/without-type-module/export-esModule-annotation-no-default.cjs b/test/js/bun/resolve/without-type-module/export-esModule-annotation-no-default.cjs new file mode 100644 index 0000000000000..32b83d4a5a939 --- /dev/null +++ b/test/js/bun/resolve/without-type-module/export-esModule-annotation-no-default.cjs @@ -0,0 +1 @@ +exports.__esModule = true; diff --git a/test/js/bun/resolve/without-type-module/export-esModule-annotation.cjs b/test/js/bun/resolve/without-type-module/export-esModule-annotation.cjs new file mode 100644 index 0000000000000..bc0625a0c4c10 --- /dev/null +++ b/test/js/bun/resolve/without-type-module/export-esModule-annotation.cjs @@ -0,0 +1,2 @@ +exports.default = true; +exports.__esModule = true; diff --git a/test/js/bun/resolve/without-type-module/export-esModule-no-annotation.cjs b/test/js/bun/resolve/without-type-module/export-esModule-no-annotation.cjs new file mode 100644 index 0000000000000..a4b65815feddd --- /dev/null +++ b/test/js/bun/resolve/without-type-module/export-esModule-no-annotation.cjs @@ -0,0 +1 @@ +exports.default = true; diff --git a/test/js/bun/resolve/without-type-module/package.json b/test/js/bun/resolve/without-type-module/package.json new file mode 100644 index 0000000000000..5b290db1c7fd3 --- /dev/null +++ b/test/js/bun/resolve/without-type-module/package.json @@ -0,0 +1,4 @@ +{ + "name": "without-type-module", + "type": "commonjs" +}