iframe-proxy

lohart13 · 2024-10-17T22:41:50Z

Pull Request Checklist

Have you added new tests to prevent regressions?
If a documentation update is necessary, have you opened a PR to the documentation repository?
Did you update the typescript typings accordingly (if applicable)?
Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
Does the name of your PR follow our conventions?

Description of Changes

This PR implements the ParameterStyle changes in #17131 which are required for #16988 and #17063

List of Breaking Changes

The bindParam option have been replaced with parameterStyle which defaults to ParameterStyle.BIND.

Summary by CodeRabbit

New Features
- Added configurable parameter binding via ParameterStyle (BIND or REPLACEMENT), exposed on Sequelize and core exports.
- insertQuery/updateQuery now return a BoundQuery (query with optional bind object).
Refactor
- Removed the bindParam option; providing it now throws. Use parameterStyle instead.
- Consolidated QueryTypes, IndexHints, and TableHints under a unified enums export; public imports updated accordingly.
Tests
- Updated tests to use ParameterStyle and BoundQuery, and to assert errors for deprecated bindParam usage.

coderabbitai · 2025-10-04T08:34:12Z

Walkthrough

Consolidates enums into a new enums module and updates imports accordingly. Replaces the bindParam option with a ParameterStyle-driven API, adds ParameterOptions and BoundQuery types, and introduces createBindParamGenerator. Updates query generators (core, db2, sqlite3), types, and tests to the new binding mechanism; removes deprecated enums and helpers.

Changes

Cohort / File(s)	Summary
Binding API refactor `packages/core/src/abstract-dialect/query-generator.js`, `.../query-generator.d.ts`, `.../query-generator-typescript.ts`, `.../query-generator.types.ts`, `.../where-sql-builder.ts`, `packages/core/src/utils/sql.ts`, `packages/core/src/sequelize.js`, `packages/core/src/sequelize.d.ts`	Replace deprecated bindParam with ParameterStyle and ParameterOptions; add BoundQuery; add createBindParamGenerator; throw on bindParam usage; return BoundQuery from insert/update; minor where-builder option mutation; expose Sequelize.ParameterStyle; normalize types to Record.
Enums consolidation and re-exports `packages/core/src/enums.ts`, `packages/core/src/index.d.ts`, `packages/core/src/index.mjs`, `packages/core/src/index-hints.ts`, `packages/core/src/table-hints.ts`, `packages/core/src/query-types.ts`	Introduce enums module exporting IndexHints, TableHints, QueryTypes, ParameterStyle; re-export from index; remove legacy enum files.
Import path alignments to enums `packages/core/src/abstract-dialect/query-interface-internal.ts`, `.../query-interface-typescript.ts`, `.../query-interface.d.ts`, `.../query-interface.js`, `.../query.d.ts`, `.../query.js`, `packages/core/src/model.d.ts`, `packages/core/src/model.js`	Update imports to use enums; replace inline index signature types with Record in Replaceable; no logic changes.
Dialect integrations `packages/db2/src/query-generator.js`, `packages/sqlite3/src/query-generator.js`, `packages/ibmi/src/query-generator.js`	Adopt ParameterStyle-based binding; add createBindParamGenerator; conditionally attach bind; remove legacy/commented bindParam helpers.
Tests updated to new API `packages/core/test/support.ts`, `packages/core/test/unit/query-generator/insert-query.test.ts`, `.../update-query.test.ts`	Use BoundQuery in helpers; import and use ParameterStyle; update expectations; assert error on bindParam option.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant QG as QueryGenerator
  participant Utils as Utils (createBindParamGenerator)
  participant Dialect as Dialect (db2/sqlite3)

  User->>QG: insertQuery/updateQuery(values, options)
  alt options.parameterStyle == BIND
    QG->>QG: init bind = {}, select BIND mode
    QG->>Utils: createBindParamGenerator(bind)
    Utils-->>QG: bindParam(value) => "$sequelize_n"
    QG->>QG: build SQL using placeholders
    QG-->>User: { query, bind }
  else options.parameterStyle == REPLACEMENT
    QG->>QG: build SQL with replacements
    QG-->>User: { query }
  end
  Note over QG: If deprecated bindParam in options → throw
  par Dialect-specific
    User->>Dialect: updateQuery(values, options)
    alt BIND
      Dialect->>Utils: createBindParamGenerator(bind)
      Dialect-->>User: { query, bind }
    else REPLACEMENT
      Dialect-->>User: { query }
    end
  end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

I twitch my ears at enums new,
Hop through binds with styles true.
No more bindParam’s dusty trail,
ParameterStyle sets the sail.
With BoundQuery carrots in a row,
I nibble SQL, watch it flow.
Thump—tests pass! Onward we go. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (2)

packages/core/src/utils/sql.ts (1)
551-563: Seed the generator counter when reusing a populated bind map.

If the caller hands in a bind object that already contains sequelize_… entries (e.g. when augmenting an existing map), restarting the counter from zero will overwrite previously collected values. Please initialize the counter from the existing maximum (or assert the map is empty) before returning the closure.
 export function createBindParamGenerator(
   bind: Record<string, unknown>,
 ): (value: unknown) => string {
-  let i = 0;
+  let i = Object.keys(bind).reduce((max, key) => {
+    if (!key.startsWith('sequelize_')) {
+      return max;
+    }
+
+    const parsed = Number.parseInt(key.slice('sequelize_'.length), 10);
+
+    return Number.isNaN(parsed) ? max : Math.max(max, parsed);
+  }, 0);
 
   return (value: unknown): string => {
     const bindName = `sequelize_${++i}`;
packages/db2/src/query-generator.js (1)
368-407: Respect super.updateQuery’s parameter-style decision before rebuilding the LIMIT variant

Here we recompute the style from options (defaulting to BIND). If super.updateQuery already had to fall back to replacements (it returns { query } without a bind object, e.g. when searchPath / prependSearchPath forces multi-statement execution), this branch reintroduces bind parameters and ends up returning a bind bag again. That regresses the very scenario super just handled.

Derive the effective parameterStyle from the response of super.updateQuery before deciding whether to allocate bind / bindParam:
-    let bind;
-    let bindParam;
-    const parameterStyle = options?.parameterStyle ?? ParameterStyle.BIND;
+    let bind;
+    let bindParam;
+    let parameterStyle =
+      options?.parameterStyle ??
+      (sql.bind == null ? ParameterStyle.REPLACEMENT : ParameterStyle.BIND);
+
+    if (sql.bind == null && parameterStyle === ParameterStyle.BIND) {
+      parameterStyle = ParameterStyle.REPLACEMENT;
+    }
This keeps the LIMIT rewrite in lock-step with the base implementation and avoids resurrecting binds in contexts that already required inlined replacements.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9b9a1d7 and 78375d2.

📒 Files selected for processing (28)

packages/core/src/abstract-dialect/query-generator-typescript.ts (3 hunks)
packages/core/src/abstract-dialect/query-generator.d.ts (4 hunks)
packages/core/src/abstract-dialect/query-generator.js (7 hunks)
packages/core/src/abstract-dialect/query-generator.types.ts (2 hunks)
packages/core/src/abstract-dialect/query-interface-internal.ts (1 hunks)
packages/core/src/abstract-dialect/query-interface-typescript.ts (1 hunks)
packages/core/src/abstract-dialect/query-interface.d.ts (1 hunks)
packages/core/src/abstract-dialect/query-interface.js (1 hunks)
packages/core/src/abstract-dialect/query.d.ts (1 hunks)
packages/core/src/abstract-dialect/query.js (1 hunks)
packages/core/src/abstract-dialect/where-sql-builder.ts (1 hunks)
packages/core/src/enums.ts (1 hunks)
packages/core/src/index-hints.ts (0 hunks)
packages/core/src/index.d.ts (1 hunks)
packages/core/src/index.mjs (2 hunks)
packages/core/src/model.d.ts (1 hunks)
packages/core/src/model.js (1 hunks)
packages/core/src/query-types.ts (0 hunks)
packages/core/src/sequelize.d.ts (1 hunks)
packages/core/src/sequelize.js (2 hunks)
packages/core/src/table-hints.ts (0 hunks)
packages/core/src/utils/sql.ts (1 hunks)
packages/core/test/support.ts (3 hunks)
packages/core/test/unit/query-generator/insert-query.test.ts (3 hunks)
packages/core/test/unit/query-generator/update-query.test.ts (3 hunks)
packages/db2/src/query-generator.js (4 hunks)
packages/ibmi/src/query-generator.js (0 hunks)
packages/sqlite3/src/query-generator.js (4 hunks)

💤 Files with no reviewable changes (4)

packages/core/src/index-hints.ts
packages/core/src/query-types.ts
packages/ibmi/src/query-generator.js
packages/core/src/table-hints.ts

🧰 Additional context used

📓 Path-based instructions (5)

packages/**/src/**/*.js