src/util/ImportUtils.ts (1)

76-81: Consider adding error handling for JSON parsing edge cases.

While the try-catch handles parsing errors, consider logging or providing more context about parsing failures for debugging purposes.

            try {
                const parsedPackage = JSON.parse(
                    await fs.readFile(potentialPackageJson, "utf8"),
                )
                // Cache the parsed package.json object and return it
                setPackageJsonCache(paths, parsedPackage)
                return parsedPackage
            } catch (error) {
+                // Log parsing errors for debugging (consider using a proper logger)
+                console.warn(`Failed to parse package.json at ${potentialPackageJson}:`, error.message)
                // If parsing fails, we still cache null to avoid repeated attempts
                setPackageJsonCache(paths, null)
                return null
            }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

test/unit/util/import-utils.ts (2)

4-6: Consider more direct import for fs.promises.

The imports look good for the test requirements. However, the fsAsync import could be more specific since you're only using fsAsync.promises.

Consider this more direct approach:

-import fsAsync from "fs"
+import { promises as fsPromises } from "fs"

Then update the stub calls accordingly:

-const statStub = sinon.stub(fsAsync.promises, "stat")
-const readFileStub = sinon.stub(fsAsync.promises, "readFile")
+const statStub = sinon.stub(fsPromises, "stat")
+const readFileStub = sinon.stub(fsPromises, "readFile")

---

214-227: Consider verifying functionality alongside cache behavior.

The test effectively verifies that caching prevents redundant filesystem operations, but it doesn't confirm that the cached results are functionally correct.

Consider adding a functional verification after the cache assertions:

 assert.equal(
     readFileStub.callCount,
     numberOfReadFileCalls,
     "readFile should be called only during the first import",
 )
+
+// Verify that caching doesn't break functionality
+const [exports1, moduleType1] = await importOrRequireFile(filePath1)
+const [exports2, moduleType2] = await importOrRequireFile(filePath2)
+const [exports3, moduleType3] = await importOrRequireFile(filePath3)
+
+// All should have the same module type since they're in the same directory
+assert.equal(moduleType1, moduleType2, "Module types should be consistent")
+assert.equal(moduleType2, moduleType3, "Module types should be consistent")

This ensures the cache doesn't just reduce calls but also maintains correct behavior.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: alumni
PR: typeorm/typeorm#11580
File: src/util/ImportUtils.ts:58-62
Timestamp: 2025-07-26T12:22:52.215Z
Learning: In TypeORM's codebase, `importOrRequireFile` is always called with resolved file paths. For example, `DirectoryExportedClassesLoader.ts` calls `PlatformTools.pathResolve(file)` before passing the path to `importOrRequireFile`, and `PlatformTools.pathResolve()` is a wrapper around Node.js `path.resolve()`. This means the cache keys in `getNearestPackageJson` will always be absolute paths, eliminating concerns about cache consistency between relative and absolute path formats.

test/unit/util/import-utils.ts (1)

184-265: Make the cache test robust: isolate filesystem, drop hard-coded counts, and guarantee cleanup

• Writing package.json and test files into __dirname risks cross-test interference and leaves residue on failures.
• Hard-coding first-call counts (stat === 1, readFile === 1 || 2) is brittle across OS/Node versions and CI vs. local.
• Ensure teardown with try/finally, and restore Sinon even on failure.

This refactor isolates the test into a dedicated temp dir, asserts that first import hits the filesystem (without hard-coding exact counts), verifies no further calls on subsequent imports, and guarantees cleanup.

Apply this diff:

@@
-    it("Should use cache to find package.json", async () => {
-        // Create package.json if not exists
-        const packageJsonPath = path.join(__dirname, "package.json")
-        const packageJsonAlreadyExisted = fsAsync.existsSync(packageJsonPath)
-        if (!packageJsonAlreadyExisted) {
-            await fs.writeFile(
-                packageJsonPath,
-                JSON.stringify({ type: "module" }),
-                "utf8",
-            )
-        }
-
-        const statSpy = sinon.spy(fsAsync.promises, "stat")
-        const readFileSpy = sinon.spy(fsAsync.promises, "readFile")
-
-        assert.equal(
-            statSpy.callCount,
-            0,
-            "stat should not be called before importOrRequireFile",
-        )
-        assert.equal(
-            readFileSpy.callCount,
-            0,
-            "readFile should not be called before importOrRequireFile",
-        )
-
-        const filePath1 = path.join(__dirname, "file1.js")
-        const filePath2 = path.join(__dirname, "file2.js")
-        const filePath3 = path.join(__dirname, "file3.js")
-
-        await fs.writeFile(filePath1, "", "utf8")
-        await fs.writeFile(filePath2, "", "utf8")
-        await fs.writeFile(filePath3, "", "utf8")
-
-        // Trigger the first import to create the cache
-        await importOrRequireFile(filePath1)
-
-        // Get the number of calls to stat and readFile after the first import
-        const numberOfStatCalls = statSpy.callCount
-        const numberOfReadFileCalls: number = readFileSpy.callCount
-
-        assert.equal(
-            numberOfStatCalls,
-            1,
-            "stat should be called for the first import",
-        )
-
-        const isCorrectNumberOfReadFileCalls =
-            numberOfReadFileCalls === 1 || numberOfReadFileCalls === 2 // Depending on platform, it might call readFile once or twice
-        assert.equal(
-            isCorrectNumberOfReadFileCalls,
-            true,
-            "readFile should be called for the first import",
-        )
-
-        // Trigger next imports to check if cache is used
-        await importOrRequireFile(filePath2)
-        await importOrRequireFile(filePath3)
-
-        assert.equal(
-            statSpy.callCount,
-            numberOfStatCalls,
-            "stat should be called only during the first import",
-        )
-        assert.equal(
-            readFileSpy.callCount,
-            numberOfReadFileCalls,
-            "readFile should be called only during the first import",
-        )
-
-        // Clean up test files
-        await fs.unlink(filePath1)
-        await fs.unlink(filePath2)
-        await fs.unlink(filePath3)
-
-        // If package.json was created by this test, remove it
-        if (!packageJsonAlreadyExisted) {
-            await fs.unlink(packageJsonPath)
-        }
-
-        sinon.restore()
-    })
+    it("Should use cache to find package.json", async () => {
+        // Use an isolated directory to avoid interfering with other tests
+        const testDir = path.join(__dirname, "testCacheValidation")
+        await fs.rm(testDir, { recursive: true, force: true })
+        await fs.mkdir(testDir, { recursive: true })
+
+        // Create a dedicated package.json used only by this test
+        const packageJsonPath = path.join(testDir, "package.json")
+        await fs.writeFile(
+            packageJsonPath,
+            JSON.stringify({ type: "module" }),
+            "utf8",
+        )
+
+        // Spy on fs calls that getNearestPackageJson performs
+        const statSpy = sinon.spy(fsAsync.promises, "stat")
+        const readFileSpy = sinon.spy(fsAsync.promises, "readFile")
+
+        try {
+            assert.equal(
+                statSpy.callCount,
+                0,
+                "stat should not be called before importOrRequireFile",
+            )
+            assert.equal(
+                readFileSpy.callCount,
+                0,
+                "readFile should not be called before importOrRequireFile",
+            )
+
+            const filePath1 = path.join(testDir, "file1.js")
+            const filePath2 = path.join(testDir, "file2.js")
+            const filePath3 = path.join(testDir, "file3.js")
+
+            await fs.writeFile(filePath1, "", "utf8")
+            await fs.writeFile(filePath2, "", "utf8")
+            await fs.writeFile(filePath3, "", "utf8")
+
+            // Trigger the first import to create/populate the cache
+            await importOrRequireFile(filePath1)
+
+            // Record counts after the first import
+            const numberOfStatCalls = statSpy.callCount
+            const numberOfReadFileCalls = readFileSpy.callCount
+
+            // Only assert that we touched the filesystem at least once (no hard-coded counts)
+            assert.ok(
+                numberOfStatCalls > 0,
+                "stat should be called during the first import",
+            )
+            assert.ok(
+                numberOfReadFileCalls > 0,
+                "readFile should be called during the first import",
+            )
+
+            // Subsequent imports should hit the cache (no extra fs calls)
+            await importOrRequireFile(filePath2)
+            await importOrRequireFile(filePath3)
+
+            assert.equal(
+                statSpy.callCount,
+                numberOfStatCalls,
+                "stat should not be called after the first import (cache hit)",
+            )
+            assert.equal(
+                readFileSpy.callCount,
+                numberOfReadFileCalls,
+                "readFile should not be called after the first import (cache hit)",
+            )
+        } finally {
+            sinon.restore()
+            await fs.rm(testDir, { recursive: true, force: true })
+        }
+    })

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: alumni
PR: typeorm/typeorm#11580
File: src/util/ImportUtils.ts:58-62
Timestamp: 2025-07-26T12:22:52.215Z
Learning: In TypeORM's codebase, `importOrRequireFile` is always called with resolved file paths. For example, `DirectoryExportedClassesLoader.ts` calls `PlatformTools.pathResolve(file)` before passing the path to `importOrRequireFile`, and `PlatformTools.pathResolve()` is a wrapper around Node.js `path.resolve()`. This means the cache keys in `getNearestPackageJson` will always be absolute paths, eliminating concerns about cache consistency between relative and absolute path formats.

Learnt from: alumni
PR: typeorm/typeorm#11580
File: src/util/ImportUtils.ts:58-62
Timestamp: 2025-07-26T12:22:52.215Z
Learning: In TypeORM's codebase, `importOrRequireFile` is always called with resolved file paths. For example, `DirectoryExportedClassesLoader.ts` calls `PlatformTools.pathResolve(file)` before passing the path to `importOrRequireFile`, and `PlatformTools.pathResolve()` is a wrapper around Node.js `path.resolve()`. This means the cache keys in `getNearestPackageJson` will always be absolute paths, eliminating concerns about cache consistency between relative and absolute path formats.