fix(runtime): require cache should not include unevaluated ESM modules. (#5233)

6 files changed, 40 insertions, 8 deletions

diff --git a/src/js/builtins/ImportMetaObject.ts b/src/js/builtins/ImportMetaObject.ts
index 9409bb0f1..2fd4880a6 100644
--- a/src/js/builtins/ImportMetaObject.ts
+++ b/src/js/builtins/ImportMetaObject.ts
@@ -177,13 +177,11 @@ export function createRequireCache() {
 
     ownKeys(target) {
       var array = [...$requireMap.$keys()];
-      const registryKeys = [...Loader.registry.$keys()];
-      for (const key of registryKeys) {
-        if (!array.includes(key)) {
+      for (const key of Loader.registry.$keys()) {
+        if (!array.includes(key) && Loader.registry.$get(key)?.evaluated) {
           $arrayPush(array, key);
         }
       }
-
       return array;
     },
 
@@ -193,7 +191,7 @@ export function createRequireCache() {
     },
 
     getOwnPropertyDescriptor(target, key: string) {
-      if ($requireMap.$has(key) || Loader.registry.$has(key)) {
+      if ($requireMap.$has(key) || Loader.registry.$get(key)?.evaluated) {
         return {
           configurable: true,
           enumerable: true,
diff --git a/src/js/out/WebCoreJSBuiltins.cpp b/src/js/out/WebCoreJSBuiltins.cpp
index a83ce2bd6..9b7647a4a 100644
--- a/src/js/out/WebCoreJSBuiltins.cpp
+++ b/src/js/out/WebCoreJSBuiltins.cpp
@@ -2296,9 +2296,9 @@ const char* const s_importMetaObjectInternalRequireCode = "(function (id){\"use
 const JSC::ConstructAbility s_importMetaObjectCreateRequireCacheCodeConstructAbility = JSC::ConstructAbility::CannotConstruct;
 const JSC::ConstructorKind s_importMetaObjectCreateRequireCacheCodeConstructorKind = JSC::ConstructorKind::None;
 const JSC::ImplementationVisibility s_importMetaObjectCreateRequireCacheCodeImplementationVisibility = JSC::ImplementationVisibility::Public;
-const int s_importMetaObjectCreateRequireCacheCodeLength = 978;
+const int s_importMetaObjectCreateRequireCacheCodeLength = 991;
 static const JSC::Intrinsic s_importMetaObjectCreateRequireCacheCodeIntrinsic = JSC::NoIntrinsic;
-const char* const s_importMetaObjectCreateRequireCacheCode = "(function (){\"use strict\";var moduleMap=new Map,inner={};return new Proxy(inner,{get(target,key){const entry=@requireMap.@get(key);if(entry)return entry;const esm=@Loader.registry.@get(key);if(esm\?.evaluated){const namespace=@Loader.getModuleNamespaceObject(esm.module),mod=@createCommonJSModule(key,namespace,!0);return @requireMap.@set(key,mod),mod}return inner[key]},set(target,key,value){return @requireMap.@set(key,value),!0},has(target,key){return @requireMap.@has(key)||@Loader.registry.@has(key)},deleteProperty(target,key){return moduleMap.@delete(key),@requireMap.@delete(key),@Loader.registry.@delete(key),!0},ownKeys(target){var array=[...@requireMap.@keys()];const registryKeys=[...@Loader.registry.@keys()];for(let key of registryKeys)if(!array.includes(key))@arrayPush(array,key);return array},getPrototypeOf(target){return null},getOwnPropertyDescriptor(target,key){if(@requireMap.@has(key)||@Loader.registry.@has(key))return{configurable:!0,enumerable:!0}}})})\n";
+const char* const s_importMetaObjectCreateRequireCacheCode = "(function (){\"use strict\";var moduleMap=new Map,inner={};return new Proxy(inner,{get(target,key){const entry=@requireMap.@get(key);if(entry)return entry;const esm=@Loader.registry.@get(key);if(esm\?.evaluated){const namespace=@Loader.getModuleNamespaceObject(esm.module),mod=@createCommonJSModule(key,namespace,!0);return @requireMap.@set(key,mod),mod}return inner[key]},set(target,key,value){return @requireMap.@set(key,value),!0},has(target,key){return @requireMap.@has(key)||@Loader.registry.@has(key)},deleteProperty(target,key){return moduleMap.@delete(key),@requireMap.@delete(key),@Loader.registry.@delete(key),!0},ownKeys(target){var array=[...@requireMap.@keys()];for(let key of @Loader.registry.@keys())if(!array.includes(key)&&@Loader.registry.@get(key)\?.evaluated)@arrayPush(array,key);return array},getPrototypeOf(target){return null},getOwnPropertyDescriptor(target,key){if(@requireMap.@has(key)||@Loader.registry.@get(key)\?.evaluated)return{configurable:!0,enumerable:!0}}})})\n";
 
 // main
 const JSC::ConstructAbility s_importMetaObjectMainCodeConstructAbility = JSC::ConstructAbility::CannotConstruct;
diff --git a/test/bun.lockb b/test/bun.lockb
index f212bb6c5..059054600 100755
--- a/test/bun.lockb
+++ b/test/bun.lockb
diff --git a/test/cli/run/require-cache-bug-5188.js b/test/cli/run/require-cache-bug-5188.js
new file mode 100644
index 000000000..cb9bd2099
--- /dev/null
+++ b/test/cli/run/require-cache-bug-5188.js
@@ -0,0 +1,21 @@
+const { strictEqual, ok } = require("assert");
+
+Loader.registry.set("bad", { evaluated: false });
+strictEqual(require.cache.bad, undefined);
+ok(!Object.hasOwn(require.cache, "bad"));
+ok(!Object.getOwnPropertyNames(require.cache).includes("bad"));
+
+// hard to simplify this test case, but importing this would cause require.cache.extract to be set
+require("msgpackr-extract");
+
+strictEqual(require.cache["extract"], undefined);
+ok(!Object.hasOwnProperty.call(require.cache, "extract"));
+ok(!Object.getOwnPropertyNames(require.cache).includes("extract"));
+
+for (const key of Object.keys(require.cache)) {
+  if (!require.cache[key]) {
+    throw new Error("require.cache has an undefined value that was in it's keys");
+  }
+}
+
+console.log("--pass--");
diff --git a/test/cli/run/require-cache.test.js b/test/cli/run/require-cache.test.ts
index e20470f9d..ed8e5ea38 100644
--- a/test/cli/run/require-cache.test.js
+++ b/test/cli/run/require-cache.test.ts
@@ -13,3 +13,15 @@ test("require.cache", () => {
   expect(stdout.toString().trim().endsWith("--pass--")).toBe(true);
   expect(exitCode).toBe(0);
 });
+
+// https://github.com/oven-sh/bun/issues/5188
+test("require.cache does not include unevaluated modules", () => {
+  const { stdout, exitCode } = Bun.spawnSync({
+    cmd: [bunExe(), "run", join(import.meta.dir, "require-cache-bug-5188.js")],
+    env: bunEnv,
+    stderr: "inherit",
+  });
+
+  expect(stdout.toString().trim().endsWith("--pass--")).toBe(true);
+  expect(exitCode).toBe(0);
+});
diff --git a/test/package.json b/test/package.json
index 4d7cd6606..58e66833f 100644
--- a/test/package.json
+++ b/test/package.json
@@ -9,6 +9,7 @@
   },
   "dependencies": {
     "@prisma/client": "5.1.1",
+    "@resvg/resvg-js": "2.4.1",
     "@swc/core": "1.3.38",
     "body-parser": "1.20.2",
     "comlink": "4.4.1",
@@ -20,12 +21,12 @@
     "iconv-lite": "0.6.3",
     "jest-extended": "4.0.0",
     "lodash": "4.17.21",
+    "msgpackr-extract": "3.0.2",
     "nodemailer": "6.9.3",
     "pg": "8.11.1",
     "pg-connection-string": "2.6.1",
     "postgres": "3.3.5",
     "prisma": "5.1.1",
-    "@resvg/resvg-js": "2.4.1",
     "socket.io": "4.7.1",
     "socket.io-client": "4.7.1",
     "supertest": "6.3.3",