From d3242cb571cf49dada63b209adf26d357beee2a2 Mon Sep 17 00:00:00 2001 From: Will Schurman Date: Mon, 13 Apr 2026 15:32:43 -0700 Subject: [PATCH] fix: upgrade knex and remove undefined value bindings --- .../entity-database-adapter-knex/package.json | 2 +- .../src/PostgresEntityDatabaseAdapter.ts | 2 +- .../src/SQLOperator.ts | 45 ++++++++++--------- .../src/__tests__/SQLOperator-test.ts | 31 +------------ .../src/internal/EntityKnexDataManager.ts | 17 ++++--- yarn.lock | 14 +++--- 6 files changed, 44 insertions(+), 67 deletions(-) diff --git a/packages/entity-database-adapter-knex/package.json b/packages/entity-database-adapter-knex/package.json index 1b41c986ef..ffc7089bae 100644 --- a/packages/entity-database-adapter-knex/package.json +++ b/packages/entity-database-adapter-knex/package.json @@ -27,7 +27,7 @@ }, "dependencies": { "@expo/entity": "workspace:^", - "knex": "^3.1.0" + "knex": "^3.2.9" }, "devDependencies": { "@expo/entity-testing-utils": "workspace:^", diff --git a/packages/entity-database-adapter-knex/src/PostgresEntityDatabaseAdapter.ts b/packages/entity-database-adapter-knex/src/PostgresEntityDatabaseAdapter.ts index 9a8462bdec..2b21aea46b 100644 --- a/packages/entity-database-adapter-knex/src/PostgresEntityDatabaseAdapter.ts +++ b/packages/entity-database-adapter-knex/src/PostgresEntityDatabaseAdapter.ts @@ -91,7 +91,7 @@ export class PostgresEntityDatabaseAdapter< .select() .from(tableName) .whereRaw(`(??) = ANY(?)`, [ - tableColumns[0], + tableColumns[0]!, tableTuples.map((tableTuple) => tableTuple[0]), ]), ); diff --git a/packages/entity-database-adapter-knex/src/SQLOperator.ts b/packages/entity-database-adapter-knex/src/SQLOperator.ts index 2a6f9ac704..90100cafa4 100644 --- a/packages/entity-database-adapter-knex/src/SQLOperator.ts +++ b/packages/entity-database-adapter-knex/src/SQLOperator.ts @@ -1,4 +1,5 @@ import assert from 'assert'; +import type { Knex } from 'knex'; /** * Supported SQL value types that can be safely parameterized. @@ -12,7 +13,6 @@ export type SupportedSQLValue = | Date | Buffer | bigint - | undefined // Will be treated as NULL | readonly SupportedSQLValue[] // For IN clauses and array types | Readonly<{ [key: string]: unknown }>; // For JSON/JSONB columns @@ -41,7 +41,7 @@ export class SQLFragment> { */ getKnexBindings( getColumnForField: (fieldName: keyof TFields) => string, - ): readonly SupportedSQLValue[] { + ): readonly Knex.RawBinding[] { return this.bindings.map((b) => { switch (b.type) { case 'entityField': @@ -49,7 +49,10 @@ export class SQLFragment> { case 'identifier': return b.name; case 'value': - return b.value; + // Needs a cast since bigint is supported by knex postgres dialect but not all dialects, and thus isn't included + // in the type. Because we only use the postgres dialect in this adapter, it's safe to allow it here. + // https://github.com/knex/knex/issues/5013#issuecomment-3368744254 + return b.value as Knex.RawBinding; } }); } @@ -133,8 +136,8 @@ export class SQLFragment> { * Handles all SupportedSQLValue types. */ private static formatDebugValue(value: SupportedSQLValue): string { - // Handle null and undefined - if (value === null || value === undefined) { + // Handle null + if (value === null) { return 'NULL'; } @@ -303,7 +306,7 @@ export function sql>( strings.forEach((string, i) => { sqlString += string; if (i < values.length) { - const value = values[i]; + const value = values[i]!; if (value instanceof SQLFragment) { // Handle nested SQL fragments @@ -344,7 +347,7 @@ type PickSupportedSQLValueKeys = { }[keyof T]; type PickStringValueKeys = { - [K in keyof T]: T[K] extends string | null | undefined ? K : never; + [K in keyof T]: T[K] extends string | null ? K : never; }[keyof T]; type JsonSerializable = @@ -368,13 +371,13 @@ export class SQLChainableFragment< > extends SQLFragment { /** * Generates an equality condition (`= value`). - * Automatically converts `null`/`undefined` to `IS NULL`. + * Automatically converts `null` to `IS NULL`. * * @param value - The value to compare against * @returns A {@link SQLFragment} representing the equality condition */ - eq(value: TValue | null | undefined): SQLFragment { - if (value === null || value === undefined) { + eq(value: TValue | null): SQLFragment { + if (value === null) { return this.isNull(); } return sql`${this} = ${value}`; @@ -382,13 +385,13 @@ export class SQLChainableFragment< /** * Generates an inequality condition (`!= value`). - * Automatically converts `null`/`undefined` to `IS NOT NULL`. + * Automatically converts `null` to `IS NOT NULL`. * * @param value - The value to compare against * @returns A {@link SQLFragment} representing the inequality condition */ - neq(value: TValue | null | undefined): SQLFragment { - if (value === null || value === undefined) { + neq(value: TValue | null): SQLFragment { + if (value === null) { return this.isNotNull(); } return sql`${this} != ${value}`; @@ -635,9 +638,7 @@ type ExtractFragmentFields = T extends SQLFragment ? F : never; // Conditional value types for expression overloads. // Uses SQLChainableFragment so that TExpr alone drives inference (single type param). type FragmentValueNullable = - TFragment extends SQLChainableFragment - ? TValue | null | undefined - : SupportedSQLValue; + TFragment extends SQLChainableFragment ? TValue | null : SupportedSQLValue; type FragmentValue = TFragment extends SQLChainableFragment ? TValue : SupportedSQLValue; @@ -950,7 +951,7 @@ function isNotNullHelper>( /** * Generates an equality condition (`= value`) from a fragment. - * Automatically converts `null`/`undefined` to `IS NULL`. + * Automatically converts `null` to `IS NULL`. * * @param fragment - A SQLFragment or SQLChainableFragment to compare * @param value - The value to compare against @@ -961,7 +962,7 @@ function eqHelper>( ): SQLFragment>; /** * Generates an equality condition (`= value`) from a field name. - * Automatically converts `null`/`undefined` to `IS NULL`. + * Automatically converts `null` to `IS NULL`. * * @param fieldName - The entity field name to compare * @param value - The value to compare against @@ -979,7 +980,7 @@ function eqHelper>( /** * Generates an inequality condition (`!= value`) from a fragment. - * Automatically converts `null`/`undefined` to `IS NOT NULL`. + * Automatically converts `null` to `IS NOT NULL`. * * @param fragment - A SQLFragment or SQLChainableFragment to compare * @param value - The value to compare against @@ -990,7 +991,7 @@ function neqHelper>( ): SQLFragment>; /** * Generates an inequality condition (`!= value`) from a field name. - * Automatically converts `null`/`undefined` to `IS NOT NULL`. + * Automatically converts `null` to `IS NOT NULL`. * * @param fieldName - The entity field name to compare * @param value - The value to compare against @@ -1521,12 +1522,12 @@ export const SQLExpression = { isNotNull: isNotNullHelper, /** - * Equality operator. Automatically converts null/undefined to IS NULL. + * Equality operator. Automatically converts null to IS NULL. */ eq: eqHelper, /** - * Inequality operator. Automatically converts null/undefined to IS NOT NULL. + * Inequality operator. Automatically converts null to IS NOT NULL. */ neq: neqHelper, diff --git a/packages/entity-database-adapter-knex/src/__tests__/SQLOperator-test.ts b/packages/entity-database-adapter-knex/src/__tests__/SQLOperator-test.ts index c97ce51549..7d0aff02bc 100644 --- a/packages/entity-database-adapter-knex/src/__tests__/SQLOperator-test.ts +++ b/packages/entity-database-adapter-knex/src/__tests__/SQLOperator-test.ts @@ -258,12 +258,11 @@ describe('SQLOperator', () => { }); it('handles all SupportedSQLValue types in getDebugString', () => { - const fragment = new SQLFragment('INSERT INTO test VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)', [ + const fragment = new SQLFragment('INSERT INTO test VALUES (?, ?, ?, ?, ?, ?, ?, ?)', [ { type: 'value', value: 'string' }, { type: 'value', value: 123 }, { type: 'value', value: true }, { type: 'value', value: null }, - { type: 'value', value: undefined }, { type: 'value', value: new Date('2024-01-01T00:00:00.000Z') }, { type: 'value', value: Buffer.from('hello') }, { type: 'value', value: BigInt(999) }, @@ -272,7 +271,7 @@ describe('SQLOperator', () => { const text = fragment.getDebugString(); expect(text).toBe( - "INSERT INTO test VALUES ('string', 123, TRUE, NULL, NULL, '2024-01-01T00:00:00.000Z', '\\x68656c6c6f', 999, ARRAY[1, 2, 3])", + "INSERT INTO test VALUES ('string', 123, TRUE, NULL, '2024-01-01T00:00:00.000Z', '\\x68656c6c6f', 999, ARRAY[1, 2, 3])", ); }); @@ -763,13 +762,6 @@ describe('SQLOperator', () => { expect(fragment.getKnexBindings(getColumnForField)).toEqual(['nullable_field']); }); - it('handles undefined in equality check', () => { - const fragment = SQLExpression.eq('nullableField', undefined); - - expect(fragment.sql).toBe('?? IS NULL'); - expect(fragment.getKnexBindings(getColumnForField)).toEqual(['nullable_field']); - }); - it('accepts a SQLFragment expression', () => { const fragment = SQLExpression.eq(sql`${entityField('stringField')}`, 'active'); expect(fragment.sql).toBe('?? = ?'); @@ -801,13 +793,6 @@ describe('SQLOperator', () => { expect(fragment.getKnexBindings(getColumnForField)).toEqual(['nullable_field']); }); - it('handles undefined in inequality check', () => { - const fragment = SQLExpression.neq('nullableField', undefined); - - expect(fragment.sql).toBe('?? IS NOT NULL'); - expect(fragment.getKnexBindings(getColumnForField)).toEqual(['nullable_field']); - }); - it('accepts a SQLFragment expression', () => { const fragment = SQLExpression.neq( sql`${entityField('stringField')}`, @@ -1131,12 +1116,6 @@ describe('SQLOperator', () => { expect(fragment.getKnexBindings(getColumnForField)).toEqual(['string_field']); }); - it('eq(undefined) uses IS NULL', () => { - const fragment = makeExpr(stringFieldFragment()).eq(undefined); - expect(fragment.sql).toBe('?? IS NULL'); - expect(fragment.getKnexBindings(getColumnForField)).toEqual(['string_field']); - }); - it('neq(value)', () => { const fragment = makeExpr(stringFieldFragment()).neq('deleted'); expect(fragment.sql).toBe('?? != ?'); @@ -1149,12 +1128,6 @@ describe('SQLOperator', () => { expect(fragment.getKnexBindings(getColumnForField)).toEqual(['string_field']); }); - it('neq(undefined) uses IS NOT NULL', () => { - const fragment = makeExpr(stringFieldFragment()).neq(undefined); - expect(fragment.sql).toBe('?? IS NOT NULL'); - expect(fragment.getKnexBindings(getColumnForField)).toEqual(['string_field']); - }); - it('gt(value)', () => { const fragment = makeExpr(intFieldFragment()).gt(10); expect(fragment.sql).toBe('?? > ?'); diff --git a/packages/entity-database-adapter-knex/src/internal/EntityKnexDataManager.ts b/packages/entity-database-adapter-knex/src/internal/EntityKnexDataManager.ts index f758dcf7f7..87e772bb6c 100644 --- a/packages/entity-database-adapter-knex/src/internal/EntityKnexDataManager.ts +++ b/packages/entity-database-adapter-knex/src/internal/EntityKnexDataManager.ts @@ -480,17 +480,16 @@ export class EntityKnexDataManager< baseWhere: SQLFragment | undefined, cursorCondition: SQLFragment | null, ): SQLFragment { - const conditions = [baseWhere, cursorCondition].filter((it) => !!it); - if (conditions.length === 0) { - return sql`TRUE`; + if (!baseWhere) { + return cursorCondition ?? sql`TRUE`; } - if (conditions.length === 1) { - return conditions[0]!; + + if (!cursorCondition) { + return baseWhere; } - // Wrap baseWhere in parens if combining with cursor condition - // We know we have exactly 2 conditions at this point - const [first, second] = conditions; - return sql`(${first}) AND ${second}`; + + // Wrap baseWhere in parens when combining with cursor condition + return sql`(${baseWhere}) AND ${cursorCondition}`; } private augmentOrderByIfNecessary( diff --git a/yarn.lock b/yarn.lock index b7a57646df..54aee09c4f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -902,7 +902,7 @@ __metadata: "@expo/entity": "workspace:^" "@expo/entity-testing-utils": "workspace:^" "@jest/globals": "npm:30.3.0" - knex: "npm:^3.1.0" + knex: "npm:^3.2.9" pg: "npm:8.20.0" ts-mockito: "npm:2.6.1" typescript: "npm:6.0.2" @@ -7179,9 +7179,9 @@ __metadata: languageName: node linkType: hard -"knex@npm:^3.1.0": - version: 3.1.0 - resolution: "knex@npm:3.1.0" +"knex@npm:^3.2.9": + version: 3.2.9 + resolution: "knex@npm:3.2.9" dependencies: colorette: "npm:2.0.19" commander: "npm:^10.0.0" @@ -7197,6 +7197,8 @@ __metadata: resolve-from: "npm:^5.0.0" tarn: "npm:^3.0.2" tildify: "npm:2.0.0" + peerDependencies: + pg-query-stream: ^4.14.0 peerDependenciesMeta: better-sqlite3: optional: true @@ -7208,13 +7210,15 @@ __metadata: optional: true pg-native: optional: true + pg-query-stream: + optional: true sqlite3: optional: true tedious: optional: true bin: knex: bin/cli.js - checksum: 10c0/d8a1f99fad143c6057e94759b2ae700ae661a0b0b2385f643011962ef501dcc7b32cfdb5bda66ef81283ca56f13630f47691c579ce66ad0e8128e209533c3785 + checksum: 10c0/5b7d56232243664991e3fae523246f0213e0fee9d96178a5744bb449e4a81a23cb8883f174ca36bc730f0a3112f93623368d925c79692e5a1d1cb3a0b3566df3 languageName: node linkType: hard