src/driver/mysql/MysqlDriver.ts (2)

587-601: Consider a more robust detection method for mysql2.

While the current detection logic works, it relies on undocumented properties that could change in future versions of mysql2. Consider using a more stable approach.

You could simplify and make the detection more explicit:

protected isUsingMysql2(): boolean {
-    // mysql2 can be detected by checking for specific properties that are unique to it
-    // mysql2 exports a 'version' property directly on the module
-    // Also mysql2 has different connection promise API
-    return !!(
-        this.mysql &&
-        (this.mysql.version ||
-            this.mysql.createConnectionPromise ||
-            (this.mysql.Connection &&
-                this.mysql.Connection.prototype.execute))
-    )
+    // mysql2 has a distinct module name property or we can check the loaded package
+    // Consider checking if this.options.connectorPackage is available
+    const connectorPackage = this.options.connectorPackage ?? "mysql"
+    return connectorPackage === "mysql2" || 
+           !!(this.mysql && this.mysql.version) // mysql2 exports version, mysql doesn't
}

Additionally, consider caching this result as a property since it won't change during runtime:

private _isUsingMysql2?: boolean

protected isUsingMysql2(): boolean {
    if (this._isUsingMysql2 === undefined) {
        this._isUsingMysql2 = // ... detection logic
    }
    return this._isUsingMysql2
}

---

681-689: Consider extracting the JSON parsing logic for better testability.

The nested JSON parsing logic could be extracted into a separate method for clarity and testing.

+private parseJsonValueForMysql2(value: any): any {
+    if (typeof value === "string") {
+        try {
+            return JSON.parse(value)
+        } catch {
+            // It's a string that's not valid JSON, which means mysql2
+            // already parsed it and it's just a string value
+            return value
+        }
+    }
+    // If it's not a string, mysql2 has already parsed it correctly
+    return value
+}

 prepareHydratedValue(value: any, columnMetadata: ColumnMetadata): any {
     // ... existing code ...
     } else if (columnMetadata.type === "json") {
         if (this.isUsingMysql2()) {
-            // With mysql2, only parse if it's a valid JSON string representation
-            // but not if it's already an object or a JSON primitive
-            if (typeof value === "string") {
-                try {
-                    // Try to parse it - if it fails, it's already a parsed string value
-                    const parsed = JSON.parse(value)
-                    value = parsed
-                } catch {
-                    // It's a string that's not valid JSON, which means mysql2
-                    // already parsed it and it's just a string value
-                    // Keep value as is
-                }
-            }
-            // If it's not a string, mysql2 has already parsed it correctly
+            value = this.parseJsonValueForMysql2(value)
         } else {
             // Classic mysql always returns JSON as strings
             value = typeof value === "string" ? JSON.parse(value) : value
         }
     }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/driver/mysql/MysqlDriver.ts (2)

592-599: Harden mysql2 detection when a custom options.driver is supplied.

If callers inject a driver via options.driver but don’t set connectorPackage, loadedConnectorPackage will still reflect the default (“mysql”), which can misroute JSON hydration for mysql2 and reintroduce double‑parsing. Either (a) infer from this.options.connectorPackage first, falling back to loadedConnectorPackage, or (b) require connectorPackage whenever options.driver is provided.

Apply this minimal guard to prefer the explicit option when present:

-    protected isUsingMysql2(): boolean {
-        // Check which package was actually loaded during initialization
-        return this.loadedConnectorPackage === "mysql2"
-    }
+    protected isUsingMysql2(): boolean {
+        const pkg = (this.options as any).connectorPackage ?? this.loadedConnectorPackage
+        return pkg === "mysql2"
+    }

Optionally, emit a warning when options.driver is set but connectorPackage is absent so users don’t hit subtle JSON behavior differences.

---

670-693: Connector‑aware JSON hydration works; consider simplifying to a driver‑agnostic safe parse.

Current branching is correct but depends on connector detection. You can avoid that coupling and keep behavior identical by parsing only when a string lexically looks like JSON and otherwise leaving it as‑is. This covers both mysql and mysql2 without needing isUsingMysql2().

Apply this refactor:

-        } else if (columnMetadata.type === "json") {
-            // mysql2 returns JSON values already parsed, but may still be a string
-            // if the JSON value itself is a string (e.g., "\"hello\"")
-            // mysql (classic) always returns JSON as strings that need parsing
-            if (this.isUsingMysql2()) {
-                // With mysql2, only parse if it's a valid JSON string representation
-                // but not if it's already an object or a JSON primitive
-                if (typeof value === "string") {
-                    try {
-                        // Try to parse it - if it fails, it's already a parsed string value
-                        const parsed = JSON.parse(value)
-                        value = parsed
-                    } catch {
-                        // It's a string that's not valid JSON, which means mysql2
-                        // already parsed it and it's just a string value
-                        // Keep value as is
-                    }
-                }
-                // If it's not a string, mysql2 has already parsed it correctly
-            } else {
-                // Classic mysql always returns JSON as strings
-                value = typeof value === "string" ? JSON.parse(value) : value
-            }
-        } else if (columnMetadata.type === "time") {
+        } else if (columnMetadata.type === "json") {
+            if (typeof value === "string") {
+                const s = value.trim()
+                const first = s[0], last = s[s.length - 1]
+                const looksJson =
+                    (first === "{" && last === "}") ||
+                    (first === "[" && last === "]") ||
+                    (first === '"' && last === '"') ||
+                    s === "true" || s === "false" || s === "null" ||
+                    /^-?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?$/.test(s)
+                if (looksJson) {
+                    try { value = JSON.parse(s) } catch { /* leave as-is */ }
+                }
+            }
+        } else if (columnMetadata.type === "time") {

test/functional/json/mysql-json-parsing/entity/JsonEntity.ts (1)

8-28: Type nits for stronger signal in tests.

Using any everywhere hides type regressions. Consider tightening where easy:

• jsonObject: Record<string, unknown>
• jsonArray: unknown[]
• complexJson: unknown

This remains flexible but avoids any.

-    jsonObject: any
+    jsonObject: Record<string, unknown>
@@
-    jsonArray: any[]
+    jsonArray: unknown[]
@@
-    complexJson: any
+    complexJson: unknown

test/functional/json/mysql-json-parsing/mysql-json-parsing.test.ts (2)

11-160: Tests are clear and cover core cases; consider DRYing across connectors.

Both “mysql” and “mysql2” suites duplicate the same scenarios. You could parameterize over connectorPackage and run a single table‑driven suite to reduce maintenance.

- describe("with mysql driver", () => { ... })
- describe("with mysql2 driver", () => { ... })
+ ;(["mysql", "mysql2"] as const).forEach((connectorPackage) => {
+   describe(`with ${connectorPackage} driver`, () => {
+     // create connections using connectorPackage
+     // run the same set of tests
+   })
+ })

---

311-344: Add a regression that mimics the original failure mode (string primitive serialized by connector).

To mirror issue #8319 more closely, insert a JSON string primitive via raw SQL (so storage is '"hello"') and read it back through the repository to ensure no throw and correct value.

+        it("should not throw when JSON string primitive is returned serialized by the connector", () =>
+            Promise.all(
+                connections.map(async (connection) => {
+                    const qr = connection.createQueryRunner()
+                    await qr.query(`INSERT INTO json_entity (jsonString) VALUES (CAST('\"hello\"' AS JSON))`)
+                    const repo = connection.getRepository(JsonEntity)
+                    const rows = await repo.find()
+                    expect(rows[0].jsonString).to.equal("hello")
+                    await qr.release()
+                }),
+            ))

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/driver/mysql/MysqlDriver.ts (2)

592-599: Harden mysql2 detection fallback

Relying only on loadedConnectorPackage can misclassify when users pass a direct driver with a mismatched connectorPackage. Add a runtime feature check fallback.

-    protected isUsingMysql2(): boolean {
-        // Check which package was actually loaded during initialization
-        return this.loadedConnectorPackage === "mysql2"
-    }
+    protected isUsingMysql2(): boolean {
+        // Primary signal from dependency loader
+        if (this.loadedConnectorPackage) {
+            return this.loadedConnectorPackage === "mysql2"
+        }
+        // Fallback: feature-detect mysql2 (Connection.prototype.execute exists only in mysql2)
+        const proto = this.mysql?.Connection?.prototype
+        return !!proto && typeof (proto as any).execute === "function"
+    }

---

1181-1189: Use PlatformTools logging instead of console.warn

Keeps logging consistent with the rest of the codebase.

-        if (this.options.driver && !this.options.connectorPackage) {
-            console.warn(
-                "Warning: MySQL driver instance provided directly without specifying connectorPackage. " +
-                    "This may lead to unexpected JSON parsing behavior differences between mysql and mysql2. " +
-                    "Consider explicitly setting connectorPackage: 'mysql' or 'mysql2' in your configuration.",
-            )
-        }
+        if (this.options.driver && !this.options.connectorPackage) {
+            PlatformTools.logWarn(
+                "MysqlDriver",
+                "MySQL driver instance provided directly without specifying connectorPackage. " +
+                    "This may lead to JSON parsing differences between 'mysql' and 'mysql2'. " +
+                    "Consider explicitly setting connectorPackage: 'mysql' or 'mysql2'.",
+            )
+        }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro