Runtime support for __esModule annotations (#3393)

* Runtime support for `__esModule` annotations * Ignore `__esModule` annotation when `"type": "module"` is set --------- Co-authored-by: Jarred Sumner <[email protected]>
oven-sh · Jun 26, 2023 · a732999 · a732999
1 parent 6d01e6e
commit a732999
Show file tree

Hide file tree

Showing 17 changed files with 221 additions and 15 deletions.
diff --git 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,13 +849,15 @@ 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));
 
     if (this->hasEvaluated)
         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
@@ -26,6 +26,7 @@ class JSCommonJSModule final : public JSC::JSDestructibleObject {
     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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,4 @@
+{
+  "name": "without-type-module",
+  "type": "commonjs"
+}