fix: remove require() calls that break bundlers - #12647

Description

ExpoDriver.loadDependencies() catches all errors thrown while loading expo-sqlite and always
rethrows DriverPackageNotInstalledError, which can misclassify module-evaluation/runtime or
transitive dependency failures as “install expo-sqlite.” This abnormal defensive try/catch pattern
discards the original cause, reducing debuggability and reliability and potentially misleading
users.

Code

src/driver/expo/ExpoDriver.ts[R40-48]

+        try {
+            this.sqlite =
+                this.options.driver ?? PlatformTools.load("expo-sqlite")
+        } catch {
+            throw new DriverPackageNotInstalledError(
+                "Expo SQLite",
+                "expo-sqlite",
+            )
       }

Evidence

The cited implementation wraps the expo-sqlite load in a bare catch { ... } and unconditionally
throws DriverPackageNotInstalledError, which means any exception (including initialization errors
or missing transitive dependencies) is collapsed into a single “package not installed” outcome while
the original error details are lost; this matches the compliance concern (PR Compliance ID 4) about
abnormal defensive try/catch blocks that hide underlying failures and misclassify errors.

Rule 4: Remove AI-generated noise
src/driver/expo/ExpoDriver.ts[40-48]
src/driver/expo/ExpoDriver.ts[36-55]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ExpoDriver.loadDependencies()` currently converts any error thrown while loading `expo-sqlite` into `DriverPackageNotInstalledError`, which can incorrectly report module evaluation crashes or missing transitive dependencies as if `expo-sqlite` itself is not installed and hides the real root cause.
## Issue Context
Compliance requires avoiding abnormal defensive try/catch patterns that discard underlying failures; the driver should only raise `DriverPackageNotInstalledError` when `expo-sqlite` cannot be resolved, and otherwise rethrow the original error unchanged so users can see the actual failure.
## Fix Focus Areas
- src/driver/expo/ExpoDriver.ts[36-55]
## Suggested fix
- Change the handler to `catch (err)` and only map to `DriverPackageNotInstalledError` for the specific “module not found for expo-sqlite” case (e.g., `code === "MODULE_NOT_FOUND"` and the message indicates `expo-sqlite`).
- If it is not that specific case, rethrow `err` unchanged (or preserve it as the cause) so initialization/transitive dependency errors surface correctly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The browser template implementation of PlatformTools.load() now calls require(name) even though
it is intended for browser builds, making it inconsistent with the rest of the browser-only stubs
and likely to fail or break bundlers at build/runtime due to the dynamic, variable-based require.
Because this template is copied into the published browser build and selected via the package.json
browser field mapping, the change impacts real consumer builds and introduces an
abnormal/inconsistent browser-platform pattern.

Code

src/platform/BrowserPlatformTools.template[R35-54]

+        const KNOWN_MODULES = [
+            // Expo
+            "expo-sqlite",
+            // React Native
+            "react-native-sqlite-storage",
+            // sql.js
+            "sql.js",
+            // redis
+            "redis",
+            "ioredis",
+        ]
+
+        if (!KNOWN_MODULES.includes(name)) {
+            throw new TypeError(
+                `Invalid Package for PlatformTools.load: ${name}`,
           )
+        }

-        return ""
+        return require(name)
   }

Evidence

The cited compliance requirement flags inconsistent/abnormal code patterns, and the browser
replacement template (src/platform/BrowserPlatformTools.template) now deviates from the prior
browser-only stub/erroring pattern by introducing require(name). The evidence further explains
that the build pipeline copies src/platform/*.template into the browser build (via the gulp
template-copy step), and the published package’s browser field maps consumers to the resulting
BrowserPlatformTools implementation, so the new dynamic require(name) is not isolated to
templates but is actually used in shipped browser builds where bundlers cannot statically resolve
variable specifiers to include needed packages.

Rule 4: Remove AI-generated noise
src/platform/BrowserPlatformTools.template[34-54]
src/platform/BrowserPlatformTools.template[10-54]
gulpfile.ts[27-52]
package.json[48-71]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`BrowserPlatformTools.template` is intended for browser builds, but `BrowserPlatformTools.load()` now performs a dynamic `require(name)` where `name` is a variable. This is inconsistent with the browser platform stubs and undermines bundler static analysis, which can cause missing modules or build/runtime failures in real consumer browser bundles.
## Issue Context
- Compliance guidance requires avoiding inconsistent/abnormal code patterns introduced by changes; adding dynamic `require(name)` to the browser stub deviates from the browser-only erroring/replacement behavior.
- The build pipeline copies `src/platform/*.template` into the browser build (gulp template-copy), and the published package selects the browser implementation through the `browser` field mapping (e.g., mapping `./browser/platform/PlatformTools.js` to `./browser/platform/BrowserPlatformTools.js`). Therefore, this template change affects the actual shipped browser build.
- Prefer an implementation that bundlers can statically analyze (e.g., a `switch (name)` or static map with literal `require("expo-sqlite")`, `require("react-native-sqlite-storage")`, etc.), and optionally guard for environments without `require` (e.g., `if (typeof require !== "function") throw ...`) to provide a clear error.
## Fix Focus Areas
- src/platform/BrowserPlatformTools.template[30-54]
- gulpfile.ts[27-52]
- package.json[48-71]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

PlatformTools.load now calls require(name) where name is a runtime variable, which
bundlers/packagers cannot statically analyze to include the right driver dependency. Any driver
using PlatformTools.load can fail to bundle or ship with missing dependencies.

Code

src/platform/PlatformTools.ts[R74-91]

+        if (!KNOWN_MODULES.includes(name)) {
+            throw new TypeError(
+                `Invalid Package for PlatformTools.load: ${name}`,
+            )
+        }
+
       // if name is not absolute or relative, then try to load package from the node_modules of the directory we are currently in
       // this is useful when we are using typeorm package globally installed and it accesses drivers
       // that are not installed globally
-
       try {
-            // switch case to explicit require statements for webpack compatibility.
-            switch (name) {
-                /**
-                 * spanner
-                 */
-                case "spanner":
-                    return require("@google-cloud/spanner")
-
-                /**
-                 * mongodb
-                 */
-                case "mongodb":
-                    return require("mongodb")
-
-                /**
-                 * hana
-                 */
-                case "@sap/hana-client":
-                    return require("@sap/hana-client")
-
-                case "@sap/hana-client/extension/Stream":
-                    return require("@sap/hana-client/extension/Stream")
-
-                /**
-                 * mysql
-                 */
-                case "mysql2":
-                    return require("mysql2")
-
-                /**
-                 * oracle
-                 */
-                case "oracledb":
-                    return require("oracledb")
-
-                /**
-                 * postgres
-                 */
-                case "pg":
-                    return require("pg")
-
-                case "pg-native":
-                    return require("pg-native")
-
-                case "pg-query-stream":
-                    return require("pg-query-stream")
-
-                case "typeorm-aurora-data-api-driver":
-                    return require("typeorm-aurora-data-api-driver")
-
-                /**
-                 * redis
-                 */
-                case "redis":
-                    return require("redis")
-
-                case "ioredis":
-                    return require("ioredis")
-
-                /**
-                 * better-sqlite3
-                 */
-                case "better-sqlite3":
-                    return require("better-sqlite3")
-
-                /**
-                 * sql.js
-                 */
-                case "sql.js":
-                    return require("sql.js")
-
-                /**
-                 * sqlserver
-                 */
-                case "mssql":
-                    return require("mssql")
-
-                /**
-                 * react-native-sqlite
-                 */
-                case "react-native-sqlite-storage":
-                    return require("react-native-sqlite-storage")
-            }
-        } catch (err) {
+            // eslint-disable-next-line @typescript-eslint/no-require-imports
+            return require(name)
+        } catch {
+            // eslint-disable-next-line @typescript-eslint/no-require-imports
           return require(
               path.resolve(process.cwd() + "/node_modules/" + name),
           )
       }
-
-        // If nothing above matched and we get here, the package was not listed within PlatformTools
-        // and is an Invalid Package.  To make it explicit that this is NOT the intended use case for
-        // PlatformTools.load - it's not just a way to replace `require` all willy-nilly - let's throw
-        // an error.
-        throw new TypeError(`Invalid Package for PlatformTools.load: ${name}`)

Evidence

The new implementation performs require(name) where name is not a string literal, making
dependency discovery non-static; this method is the shared loader for driver packages.

src/platform/PlatformTools.ts[40-92]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`PlatformTools.load()` uses `require(name)` with a variable module specifier. Even with an allowlist, many bundlers rely on static string-literal `require("...")` calls to detect and include optional dependencies.
## Issue Context
`PlatformTools.load()` is the central dependency loader used by many drivers (pg, mongodb, expo, react-native, etc.), so a dynamic require here can break bundling for multiple targets.
## Fix Focus Areas
- src/platform/PlatformTools.ts[40-92]
## Suggested fix
- Replace `require(name)` with a static loader map or `switch(name)` that contains literal `require("pkg")` calls for each allowed module.
- Keep the existing fallback `require(path.resolve(process.cwd() + "/node_modules/" + name))` for global installs, but only after a literal require attempt.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

PlatformTools.load() introduces a local constant named KNOWN_MODULES, which is inconsistent with
the file’s existing camelCase local naming style. This deviation reduces readability and breaks the
project’s naming conventions.

Code

src/platform/PlatformTools.ts[41]

+        const KNOWN_MODULES = [

Evidence

PR Compliance ID 1 requires new identifiers to follow established naming standards. In
PlatformTools.load(), the new function-local identifier KNOWN_MODULES deviates from the file’s
otherwise camelCase local naming pattern.

Rule 1: Consistent Naming Conventions
src/platform/PlatformTools.ts[40-78]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`PlatformTools.load()` defines a local constant `KNOWN_MODULES`, which does not match the file’s established local identifier naming style.

## Issue Context
Most local variables in `PlatformTools.ts` use camelCase (e.g., `hashFunction`, `normalizedPath`). A screaming-constant name for a function-local value is inconsistent with the surrounding codebase conventions.

## Fix Focus Areas
- src/platform/PlatformTools.ts[40-78]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

ExpoDriver.loadDependencies() comment claims it loads dependencies via require, but the
implementation uses PlatformTools.load("expo-sqlite"). This misleading comment is noise that can
confuse maintainers and reviewers.

Code

src/driver/expo/ExpoDriver.ts[R36-38]

    /**
-     * Loads the `expo-sqlite` module when `options.driver` is not set. Extracted
-     * as a method so tests can stub the module resolution without depending on
-     * whether `expo-sqlite` happens to be installed in the workspace.
-     *
-     * A literal `require("expo-sqlite")` is used so Metro's static bundler picks
-     * the dependency up at build time — `PlatformTools.load()` is a throwing
-     * stub in the browser/React-Native build and cannot be used here.
-     */
-    protected requireExpoSqlite(): unknown {
-        return require("expo-sqlite")
-    }
-
-    /**
-     * If driver dependency is not given explicitly, resolve it via
-     * `requireExpoSqlite()` and validate that the loaded module exposes the
-     * modern async API introduced in Expo SDK v52.
+     * If driver dependency is not given explicitly, then try to load it via "require".
     */

Evidence

PR Compliance ID 4 requires avoiding AI-like/noisy additions, including inconsistent or misleading
comments. The added comment explicitly references require while the code now calls
PlatformTools.load, making the comment inaccurate.

Rule 4: Remove AI-generated noise
src/driver/expo/ExpoDriver.ts[36-43]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new doc comment in `ExpoDriver` says the dependency is loaded via `require`, but the code calls `PlatformTools.load("expo-sqlite")`.

## Issue Context
Inaccurate comments are considered “noise” under the compliance checklist because they misrepresent behavior and increase maintenance burden.

## Fix Focus Areas
- src/driver/expo/ExpoDriver.ts[36-43]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

ExpoDriver.loadDependencies() now always throws a TypeORMError instructing users to upgrade to Expo
SDK v52+ when openDatabaseAsync is missing, even if the user explicitly provided a custom driver
override. This can misdiagnose invalid custom driver wrappers/mocks as an Expo SDK version problem
and makes failures harder to debug.

Code

src/driver/expo/ExpoDriver.ts[R50-55]

+        // The modern Expo SQLite API that TypeORM supports exposes `openDatabaseAsync` as a function
+        if (typeof this.sqlite.openDatabaseAsync !== "function") {
            throw new TypeORMError(
-                `Legacy Expo SQLite driver is not supported — ${hint}.`,
+                `The provided Expo SQLite client is not supported. Please upgrade your Expo SDK to v52 or higher!`,
            )
        }

Evidence

The options explicitly allow passing a custom driver, but the current error message always blames
Expo SDK version when openDatabaseAsync is missing, even for custom overrides.

src/driver/expo/ExpoDataSourceOptions.ts[17-23]
src/driver/expo/ExpoDriver.ts[36-55]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ExpoDriver.loadDependencies()` emits a single generic message when `openDatabaseAsync` is missing, regardless of whether the user passed a custom `options.driver` or TypeORM auto-loaded `expo-sqlite`. This misleads users who provide a custom driver (wrapper/mock) that doesn’t match the expected surface.

## Issue Context
`ExpoDataSourceOptions` explicitly supports passing a custom `driver` instance/module, so the error should distinguish between:
- an invalid custom override vs
- an outdated `expo-sqlite` / Expo SDK.

## Fix Focus Areas
- src/driver/expo/ExpoDriver.ts[39-55]
- src/driver/expo/ExpoDataSourceOptions.ts[17-23]

## Suggested fix
Reintroduce differentiated messaging, e.g.:
- If `this.options.driver` is provided and `openDatabaseAsync` is missing: instruct to ensure the provided driver exposes `openDatabaseAsync`.
- Otherwise: instruct to upgrade to Expo SDK v52+ / `expo-sqlite` with the async API.
Optionally keep the message text compatible with prior behavior to avoid breaking downstream assertions/docs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The PR deletes the unit tests that exercised ExpoDriver.loadDependencies() behavior (custom driver
override and various error paths), leaving the changed dependency-loading logic without targeted
regression coverage. This increases the risk of future breaking changes around Expo module loading
and API validation going undetected.

Code

test/unit/driver/expo-driver.test.ts[L1-127]

-import { expect } from "chai"
-import { ExpoDriver } from "../../../src/driver/expo/ExpoDriver"
-import type { ExpoDataSourceOptions } from "../../../src/driver/expo/ExpoDataSourceOptions"
-import {
-    DriverPackageNotInstalledError,
-    TypeORMError,
-} from "../../../src/error"
-
-// Minimal driver module shape matching the Expo SDK v52+ async surface.
-// `loadDependencies()` only inspects `openDatabaseAsync`.
-const modernDriver = {
-    openDatabaseAsync: () => undefined,
-}
-
-const moduleNotFoundError = (): Error & { code?: string } => {
-    const err = new Error("Cannot find module 'expo-sqlite'") as Error & {
-        code?: string
-    }
-    err.code = "MODULE_NOT_FOUND"
-    return err
-}
-
-// Subclass that lets each test control what `require("expo-sqlite")` returns
-// without touching Node's module resolver. The real base-class constructor
-// calls `loadDependencies` eagerly, so tests build instances via `Object.create`
-// rather than `new` — see `build()` below.
-class TestableExpoDriver extends ExpoDriver {
-    declare public sqlite: unknown
-    declare public onRequire: () => unknown
-
-    protected requireExpoSqlite(): unknown {
-        return this.onRequire()
-    }
-
-    public loadDependenciesForTest(): void {
-        this.loadDependencies()
-    }
-}
-
-const build = (
-    driverOption: ExpoDataSourceOptions["driver"],
-    onRequire?: () => unknown,
-): TestableExpoDriver => {
-    const driver = Object.create(
-        TestableExpoDriver.prototype,
-    ) as TestableExpoDriver
-    Object.assign(driver, {
-        options: { type: "expo", database: ":memory:", driver: driverOption },
-        onRequire:
-            onRequire ??
-            (() => {
-                throw moduleNotFoundError()
-            }),
-    })
-    return driver
-}
-
-describe("driver > expo > loadDependencies", () => {
-    it("accepts an explicit driver that exposes the modern async API", () => {
-        const driver = build(modernDriver)
-        driver.loadDependenciesForTest()
-        expect(driver.sqlite).to.equal(modernDriver)
-    })
-
-    it("throws TypeORMError when the user-supplied driver lacks `openDatabaseAsync`", () => {
-        const driver = build({ openDatabase: () => undefined })
-        expect(() => driver.loadDependenciesForTest())
-            .to.throw(TypeORMError)
-            .with.property("message")
-            .that.matches(/custom overrides must match/)
-    })
-
-    it("throws TypeORMError with a SDK upgrade hint when `expo-sqlite` is installed but stale", () => {
-        const staleModule = { openDatabase: () => undefined }
-        const driver = build(undefined, () => staleModule)
-        expect(() => driver.loadDependenciesForTest())
-            .to.throw(TypeORMError)
-            .with.property("message")
-            .that.matches(/Expo SDK v52 or later/)
-    })
-
-    it("loads the module when `requireExpoSqlite()` resolves", () => {
-        const driver = build(undefined, () => modernDriver)
-        driver.loadDependenciesForTest()
-        expect(driver.sqlite).to.equal(modernDriver)
-    })
-
-    it("throws DriverPackageNotInstalledError when the module is not found", () => {
-        const driver = build(undefined)
-        expect(() => driver.loadDependenciesForTest())
-            .to.throw(DriverPackageNotInstalledError)
-            .with.property("message")
-            .that.includes("expo-sqlite")
-    })
-
-    it("re-throws non-MODULE_NOT_FOUND errors unchanged", () => {
-        const boom = new Error("expo-sqlite crashed during initialization")
-        const driver = build(undefined, () => {
-            throw boom
-        })
-        expect(() => driver.loadDependenciesForTest()).to.throw(
-            "expo-sqlite crashed during initialization",
-        )
-    })
-
-    it("re-throws MODULE_NOT_FOUND errors unchanged when a different module is missing", () => {
-        // A transitive dependency of `expo-sqlite` raises MODULE_NOT_FOUND
-        // with the real Node message shape — first line names the missing
-        // module, followed by a `Require stack:` that mentions every caller
-        // including `expo-sqlite/index.js`. Matching against the whole
-        // message would falsely claim the user needs to install expo-sqlite;
-        // only the first line identifies the actually-missing module.
-        const err = new Error(
-            "Cannot find module 'react-native-get-random-values'\n" +
-                "Require stack:\n" +
-                "- /node_modules/expo-sqlite/src/index.ts\n" +
-                "- /app/index.ts",
-        ) as Error & { code?: string }
-        err.code = "MODULE_NOT_FOUND"
-        const driver = build(undefined, () => {
-            throw err
-        })
-        expect(() => driver.loadDependenciesForTest()).to.throw(
-            "react-native-get-random-values",
-        )
-    })
-})

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR removes the ExpoDriver unit test suite, reducing regression coverage for dependency loading and API surface validation.

## Issue Context
This PR materially changes how the Expo driver loads dependencies (switching to `PlatformTools.load(...)` and adjusting error behavior). Tests that previously covered these cases were deleted, but no replacement tests were added.

## Fix Focus Areas
- src/driver/expo/ExpoDriver.ts[36-56]

## Suggested fix
Add a new unit test file (e.g. `test/unit/driver/expo-driver.test.ts`) that covers at least:
- Accepting an explicit `options.driver` with `openDatabaseAsync`
- Throwing a clear TypeORMError when a custom driver lacks `openDatabaseAsync`
- Throwing `DriverPackageNotInstalledError` when `expo-sqlite` cannot be loaded
- (If applicable) ensuring non-install-related load failures are not misclassified (depending on intended behavior)

If stubbing `PlatformTools.load` is difficult, consider reinstating an injectable/overridable module-loader method (like the prior `requireExpoSqlite()` pattern) specifically to keep this behavior testable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

🐞 Bugs (1) 📘 Rule violations (2) 📜 Skill insights (0)