diff options
17 files changed, 221 insertions, 15 deletions
diff --git a/src/bun.js/bindings/CommonJSModuleRecord.cpp b/src/bun.js/bindings/CommonJSModuleRecord.cpp index 3615db774..c7dac89c2 100644 --- a/src/bun.js/bindings/CommonJSModuleRecord.cpp +++ b/src/bun.js/bindings/CommonJSModuleRecord.cpp @@ -586,13 +586,41 @@ void JSCommonJSModule::toSyntheticSource(JSC::JSGlobalObject* globalObject, auto result = this->exportsObject(); auto& vm = globalObject->vm(); - exportNames.append(vm.propertyNames->defaultKeyword); - exportValues.append(result); // This exists to tell ImportMetaObject.ts that this is a CommonJS module. exportNames.append(Identifier::fromUid(vm.symbolRegistry().symbolForKey("CommonJS"_s))); exportValues.append(jsNumber(0)); + // Bun's intepretation of the "__esModule" annotation: + // + // - If a "default" export does not exist OR the __esModule annotation is not present, then we + // set the default export to the exports object + // + // - If a "default" export also exists, then we set the default export + // to the value of it (matching Babel behavior) + // + // https://stackoverflow.com/questions/50943704/whats-the-purpose-of-object-definepropertyexports-esmodule-value-0 + // https://github.com/nodejs/node/issues/40891 + // https://github.com/evanw/bundler-esm-cjs-tests + // https://github.com/evanw/esbuild/issues/1591 + // https://github.com/oven-sh/bun/issues/3383 + // + // Note that this interpretation is slightly different + // + // - We do not ignore when "type": "module" or when the file + // extension is ".mjs". Build tools determine that based on the + // caller's behavior, but in a JS runtime, there is only one ModuleNamespaceObject. + // + // It would be possible to match the behavior at runtime, but + // it would need further engine changes which do not match the ES Module spec + // + // - We ignore the value of the annotation. We only look for the + // existence of the value being set. This is for performance reasons, but also + // this annotation is meant for tooling and the only usages of setting + // it to something that does NOT evaluate to "true" I could find were in + // unit tests of build tools. Happy to revisit this if users file an issue. + bool needsToAssignDefault = true; + if (result.isObject()) { auto* exports = asObject(result); @@ -601,21 +629,78 @@ void JSCommonJSModule::toSyntheticSource(JSC::JSGlobalObject* globalObject, exportNames.reserveCapacity(size + 2); exportValues.ensureCapacity(size + 2); - if (canPerformFastEnumeration(structure)) { + auto catchScope = DECLARE_CATCH_SCOPE(vm); + + Identifier esModuleMarker = builtinNames(vm).__esModulePublicName(); + bool hasESModuleMarker = !this->ignoreESModuleAnnotation && exports->hasProperty(globalObject, esModuleMarker); + if (catchScope.exception()) { + catchScope.clearException(); + } + + if (hasESModuleMarker) { + if (canPerformFastEnumeration(structure)) { + exports->structure()->forEachProperty(vm, [&](const PropertyTableEntry& entry) -> bool { + auto key = entry.key(); + if (key->isSymbol() || entry.attributes() & PropertyAttribute::DontEnum || key == esModuleMarker) + return true; + + needsToAssignDefault = needsToAssignDefault && key != vm.propertyNames->defaultKeyword; + + JSValue value = exports->getDirect(entry.offset()); + exportNames.append(Identifier::fromUid(vm, key)); + exportValues.append(value); + return true; + }); + } else { + JSC::PropertyNameArray properties(vm, JSC::PropertyNameMode::Strings, JSC::PrivateSymbolMode::Exclude); + exports->methodTable()->getOwnPropertyNames(exports, globalObject, properties, DontEnumPropertiesMode::Exclude); + if (catchScope.exception()) { + catchScope.clearExceptionExceptTermination(); + return; + } + + for (auto property : properties) { + if (UNLIKELY(property.isEmpty() || property.isNull() || property == esModuleMarker || property.isPrivateName() || property.isSymbol())) + continue; + + // ignore constructor + if (property == vm.propertyNames->constructor) + continue; + + JSC::PropertySlot slot(exports, PropertySlot::InternalMethodType::Get); + if (!exports->getPropertySlot(globalObject, property, slot)) + continue; + + exportNames.append(property); + + JSValue getterResult = slot.getValue(globalObject, property); + + // If it throws, we keep them in the exports list, but mark it as undefined + // This is consistent with what Node.js does. + if (catchScope.exception()) { + catchScope.clearException(); + getterResult = jsUndefined(); + } + + exportValues.append(getterResult); + + needsToAssignDefault = needsToAssignDefault && property != vm.propertyNames->defaultKeyword; + } + } + + } else if (canPerformFastEnumeration(structure)) { exports->structure()->forEachProperty(vm, [&](const PropertyTableEntry& entry) -> bool { auto key = entry.key(); - if (key->isSymbol() || key == vm.propertyNames->defaultKeyword || entry.attributes() & PropertyAttribute::DontEnum) + if (key->isSymbol() || entry.attributes() & PropertyAttribute::DontEnum || key == vm.propertyNames->defaultKeyword) return true; - exportNames.append(Identifier::fromUid(vm, key)); - JSValue value = exports->getDirect(entry.offset()); + exportNames.append(Identifier::fromUid(vm, key)); exportValues.append(value); return true; }); } else { - auto catchScope = DECLARE_CATCH_SCOPE(vm); JSC::PropertyNameArray properties(vm, JSC::PropertyNameMode::Strings, JSC::PrivateSymbolMode::Exclude); exports->methodTable()->getOwnPropertyNames(exports, globalObject, properties, DontEnumPropertiesMode::Exclude); if (catchScope.exception()) { @@ -624,11 +709,11 @@ void JSCommonJSModule::toSyntheticSource(JSC::JSGlobalObject* globalObject, } for (auto property : properties) { - if (UNLIKELY(property.isEmpty() || property.isNull() || property.isPrivateName() || property.isSymbol())) + if (UNLIKELY(property.isEmpty() || property.isNull() || property == vm.propertyNames->defaultKeyword || property.isPrivateName() || property.isSymbol())) continue; // ignore constructor - if (property == vm.propertyNames->constructor || property == vm.propertyNames->defaultKeyword) + if (property == vm.propertyNames->constructor) continue; JSC::PropertySlot slot(exports, PropertySlot::InternalMethodType::Get); @@ -650,6 +735,11 @@ void JSCommonJSModule::toSyntheticSource(JSC::JSGlobalObject* globalObject, } } } + + if (needsToAssignDefault) { + exportNames.append(vm.propertyNames->defaultKeyword); + exportValues.append(result); + } } JSValue JSCommonJSModule::exportsObject() @@ -759,6 +849,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)); @@ -766,6 +857,7 @@ bool JSCommonJSModule::evaluate( return true; this->sourceCode.set(vm, this, JSC::JSSourceCode::create(vm, WTFMove(rawInputSource))); + WTF::NakedPtr<JSC::Exception> exception; evaluateCommonJSModuleOnce(vm, globalObject, this, this->m_dirname.get(), this->m_filename.get(), exception); @@ -796,6 +888,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) { @@ -827,6 +920,8 @@ std::optional<JSC::SourceCode> 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 a96ab5f75..15792f9da 100644 --- a/src/bun.js/bindings/CommonJSModuleRecord.h +++ b/src/bun.js/bindings/CommonJSModuleRecord.h @@ -26,6 +26,7 @@ public: mutable JSC::WriteBarrier<JSC::JSString> m_dirname; mutable JSC::WriteBarrier<Unknown> m_paths; mutable JSC::WriteBarrier<JSC::JSSourceCode> 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 f77b57216..213291e7b 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 57940550f..db1e38d3e 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 5838d8a49..b25bb4b10 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/src/js/builtins/BunBuiltinNames.h b/src/js/builtins/BunBuiltinNames.h index af9291918..1897f939e 100644 --- a/src/js/builtins/BunBuiltinNames.h +++ b/src/js/builtins/BunBuiltinNames.h @@ -37,6 +37,7 @@ using namespace JSC; macro(WritableStream) \ macro(WritableStreamDefaultController) \ macro(WritableStreamDefaultWriter) \ + macro(__esModule) \ macro(_events) \ macro(abortAlgorithm) \ macro(abortSteps) \ @@ -59,11 +60,11 @@ using namespace JSC; macro(cloneArrayBuffer) \ macro(close) \ macro(closeAlgorithm) \ + macro(closeRequest) \ + macro(closeRequested) \ macro(closed) \ macro(closedPromise) \ macro(closedPromiseCapability) \ - macro(closeRequest) \ - macro(closeRequested) \ macro(code) \ macro(commonJSSymbol) \ macro(connect) \ @@ -94,6 +95,7 @@ using namespace JSC; macro(end) \ macro(errno) \ macro(errorSteps) \ + macro(evaluateCommonJSModule) \ macro(execArgv) \ macro(exports) \ macro(extname) \ @@ -137,7 +139,6 @@ using namespace JSC; macro(lazyLoad) \ macro(lazyStreamPrototypeMap) \ macro(loadCJS2ESM) \ - macro(evaluateCommonJSModule) \ macro(localStreams) \ macro(main) \ macro(makeDOMException) \ diff --git a/test/js/bun/resolve/esModule-annotation.test.js b/test/js/bun/resolve/esModule-annotation.test.js new file mode 100644 index 000000000..33c84be5d --- /dev/null +++ b/test/js/bun/resolve/esModule-annotation.test.js @@ -0,0 +1,68 @@ +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"; + +describe('without type: "module"', () => { + test("module.exports = {}", () => { + expect(WithoutTypeModuleExportEsModuleAnnotationMissingDefault.default).toEqual({}); + expect(WithoutTypeModuleExportEsModuleAnnotationMissingDefault.__esModule).toBeUndefined(); + }); + + test("exports.__esModule = true", () => { + expect(WithoutTypeModuleExportEsModuleAnnotationNoDefault.default).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;", () => { + expect(WithoutTypeModuleExportEsModuleNoAnnotation.default).toEqual({ + default: true, + }); + expect(WithoutTypeModuleExportEsModuleAnnotation.__esModule).toBeUndefined(); + }); +}); + +describe('with type: "module"', () => { + test("module.exports = {}", () => { + expect(WithTypeModuleExportEsModuleAnnotationMissingDefault.default).toEqual({}); + expect(WithTypeModuleExportEsModuleAnnotationMissingDefault.__esModule).toBeUndefined(); + }); + + 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/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 000000000..f053ebf79 --- /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/with-type-module/export-esModule-annotation-no-default.cjs b/test/js/bun/resolve/with-type-module/export-esModule-annotation-no-default.cjs new file mode 100644 index 000000000..32b83d4a5 --- /dev/null +++ b/test/js/bun/resolve/with-type-module/export-esModule-annotation-no-default.cjs @@ -0,0 +1 @@ +exports.__esModule = true; diff --git a/test/js/bun/resolve/with-type-module/export-esModule-annotation.cjs b/test/js/bun/resolve/with-type-module/export-esModule-annotation.cjs new file mode 100644 index 000000000..bc0625a0c --- /dev/null +++ b/test/js/bun/resolve/with-type-module/export-esModule-annotation.cjs @@ -0,0 +1,2 @@ +exports.default = true; +exports.__esModule = true; diff --git a/test/js/bun/resolve/with-type-module/export-esModule-no-annotation.cjs b/test/js/bun/resolve/with-type-module/export-esModule-no-annotation.cjs new file mode 100644 index 000000000..a4b65815f --- /dev/null +++ b/test/js/bun/resolve/with-type-module/export-esModule-no-annotation.cjs @@ -0,0 +1 @@ +exports.default = true; 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 000000000..f1863a426 --- /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 000000000..f053ebf79 --- /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 000000000..32b83d4a5 --- /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 000000000..bc0625a0c --- /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 000000000..a4b65815f --- /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 000000000..5b290db1c --- /dev/null +++ b/test/js/bun/resolve/without-type-module/package.json @@ -0,0 +1,4 @@ +{ + "name": "without-type-module", + "type": "commonjs" +} |