diff --git a/.changeset/add-columnmapper-validation.md b/.changeset/add-columnmapper-validation.md new file mode 100644 index 0000000000..8683e38625 --- /dev/null +++ b/.changeset/add-columnmapper-validation.md @@ -0,0 +1,5 @@ +--- +'@electric-sql/client': patch +--- + +Add runtime validation for `columnMapper` option to catch common mistake of passing the factory function instead of calling it. Provides helpful error messages like "Did you forget to call snakeCamelMapper()?" when users pass `snakeCamelMapper` instead of `snakeCamelMapper()`. diff --git a/packages/typescript-client/src/client.ts b/packages/typescript-client/src/client.ts index ac8592dc41..3aeeb2c7bb 100644 --- a/packages/typescript-client/src/client.ts +++ b/packages/typescript-client/src/client.ts @@ -14,6 +14,7 @@ import { ColumnMapper, encodeWhereClause, quoteIdentifier, + isValidColumnMapper, } from './column-mapper' import { getOffset, isUpToDateMessage, isChangeMessage } from './helpers' import { @@ -25,6 +26,7 @@ import { ReservedParamError, MissingHeadersError, StaleCacheError, + InvalidColumnMapperError, } from './error' import { BackoffDefaults, @@ -1735,7 +1737,26 @@ function validateOptions(options: Partial>): void { validateParams(options.params) - return + if (options.columnMapper !== undefined) { + const mapper = options.columnMapper as unknown + + if (typeof mapper === `function`) { + const fnName = + (mapper as { name?: string }).name || `columnMapper function` + throw new InvalidColumnMapperError( + `columnMapper must be a ColumnMapper object, not a function. ` + + `Did you forget to call ${fnName}()? ` + + `Use columnMapper: snakeCamelMapper() instead of columnMapper: snakeCamelMapper` + ) + } + + if (!isValidColumnMapper(mapper)) { + throw new InvalidColumnMapperError( + `columnMapper must be an object with encode and decode functions. ` + + `Use snakeCamelMapper() or createColumnMapper() to create a valid ColumnMapper.` + ) + } + } } // `unknown` being in the value is a bit of defensive programming if user doesn't use TS diff --git a/packages/typescript-client/src/column-mapper.ts b/packages/typescript-client/src/column-mapper.ts index c1a6d3d654..7171b390c5 100644 --- a/packages/typescript-client/src/column-mapper.ts +++ b/packages/typescript-client/src/column-mapper.ts @@ -36,6 +36,15 @@ export function quoteIdentifier(identifier: string): string { * For type conversions (e.g., string → Date), use the `parser` option. * For value transformations (e.g., encryption), use the `transformer` option. * + * **Important**: Always call the factory function to get a ColumnMapper: + * ```typescript + * // ✓ Correct - call the function + * columnMapper: snakeCamelMapper() + * + * // ✗ Wrong - passing the function itself + * columnMapper: snakeCamelMapper + * ``` + * * @example * ```typescript * const mapper = snakeCamelMapper() @@ -57,6 +66,24 @@ export interface ColumnMapper { encode: (appColumnName: AppColumnName) => DbColumnName } +/** + * Validates that a value is a valid ColumnMapper object. + * Returns false if the value is a function (common mistake of passing + * the factory without calling it) or if it lacks encode/decode methods. + * + * @param value - The value to validate + * @returns true if the value is a valid ColumnMapper, false otherwise + * @internal + */ +export function isValidColumnMapper(value: unknown): value is ColumnMapper { + if (typeof value !== `object` || value === null) { + return false + } + + const obj = value as Record + return typeof obj.encode === `function` && typeof obj.decode === `function` +} + /** * Converts a snake_case string to camelCase. * diff --git a/packages/typescript-client/src/error.ts b/packages/typescript-client/src/error.ts index 3618df06d2..7df2ca1470 100644 --- a/packages/typescript-client/src/error.ts +++ b/packages/typescript-client/src/error.ts @@ -123,3 +123,10 @@ export class StaleCacheError extends Error { this.name = `StaleCacheError` } } + +export class InvalidColumnMapperError extends Error { + constructor(message: string) { + super(message) + this.name = `InvalidColumnMapperError` + } +} diff --git a/packages/typescript-client/src/index.ts b/packages/typescript-client/src/index.ts index 56bc846e74..7dfd4264ca 100644 --- a/packages/typescript-client/src/index.ts +++ b/packages/typescript-client/src/index.ts @@ -6,7 +6,7 @@ export { isControlMessage, isVisibleInSnapshot, } from './helpers' -export { FetchError } from './error' +export { FetchError, InvalidColumnMapperError } from './error' export { type BackoffOptions, BackoffDefaults } from './fetch' export { ELECTRIC_PROTOCOL_QUERY_PARAMS } from './constants' export { @@ -15,5 +15,6 @@ export { snakeCamelMapper, snakeToCamel, camelToSnake, + isValidColumnMapper, } from './column-mapper' export { compileExpression, compileOrderBy } from './expression-compiler' diff --git a/packages/typescript-client/test/column-mapper.test.ts b/packages/typescript-client/test/column-mapper.test.ts index dfd18c8091..e4dd13eb96 100644 --- a/packages/typescript-client/test/column-mapper.test.ts +++ b/packages/typescript-client/test/column-mapper.test.ts @@ -6,6 +6,7 @@ import { snakeCamelMapper, encodeWhereClause, quoteIdentifier, + isValidColumnMapper, } from '../src/column-mapper' import type { Schema } from '../src/types' @@ -426,3 +427,55 @@ describe(`columnMapper and transformer together`, () => { }) }) }) + +describe(`isValidColumnMapper`, () => { + it(`should return true for valid ColumnMapper objects`, () => { + const mapper = snakeCamelMapper() + expect(isValidColumnMapper(mapper)).toBe(true) + }) + + it(`should return true for ColumnMapper created with createColumnMapper`, () => { + const mapper = createColumnMapper({ user_id: `userId` }) + expect(isValidColumnMapper(mapper)).toBe(true) + }) + + it(`should return true for custom ColumnMapper objects with encode and decode`, () => { + const customMapper = { + encode: (name: string) => name.toLowerCase(), + decode: (name: string) => name.toUpperCase(), + } + expect(isValidColumnMapper(customMapper)).toBe(true) + }) + + it(`should return false for functions`, () => { + expect(isValidColumnMapper(snakeCamelMapper)).toBe(false) + expect(isValidColumnMapper(createColumnMapper)).toBe(false) + expect(isValidColumnMapper(() => {})).toBe(false) + }) + + it(`should return false for null and undefined`, () => { + expect(isValidColumnMapper(null)).toBe(false) + expect(isValidColumnMapper(undefined)).toBe(false) + }) + + it(`should return false for primitive values`, () => { + expect(isValidColumnMapper(`string`)).toBe(false) + expect(isValidColumnMapper(123)).toBe(false) + expect(isValidColumnMapper(true)).toBe(false) + }) + + it(`should return false for objects missing encode`, () => { + const incomplete = { decode: (name: string) => name } + expect(isValidColumnMapper(incomplete)).toBe(false) + }) + + it(`should return false for objects missing decode`, () => { + const incomplete = { encode: (name: string) => name } + expect(isValidColumnMapper(incomplete)).toBe(false) + }) + + it(`should return false for objects with non-function encode/decode`, () => { + const invalid = { encode: `not a function`, decode: `not a function` } + expect(isValidColumnMapper(invalid)).toBe(false) + }) +}) diff --git a/packages/typescript-client/test/stream.test.ts b/packages/typescript-client/test/stream.test.ts index db2f34677d..c14c5763e3 100644 --- a/packages/typescript-client/test/stream.test.ts +++ b/packages/typescript-client/test/stream.test.ts @@ -1,6 +1,12 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { ShapeStream, isChangeMessage, Message, Row } from '../src' -import { snakeCamelMapper } from '../src/column-mapper' +import { + ShapeStream, + isChangeMessage, + Message, + Row, + InvalidColumnMapperError, +} from '../src' +import { snakeCamelMapper, createColumnMapper } from '../src/column-mapper' import { resolveInMacrotask } from './support/test-helpers' describe(`ShapeStream`, () => { @@ -407,4 +413,115 @@ describe(`ShapeStream`, () => { expect(changeMessage!.value).not.toHaveProperty(`user_id`) expect(changeMessage!.value).not.toHaveProperty(`created_at`) }) + + describe(`columnMapper validation`, () => { + it(`should throw InvalidColumnMapperError when passing snakeCamelMapper without calling it`, () => { + expect(() => { + new ShapeStream({ + url: shapeUrl, + params: { table: `foo` }, + // Common mistake: passing the function instead of calling it + // @ts-expect-error - intentionally testing invalid input + columnMapper: snakeCamelMapper, + }) + }).toThrow(InvalidColumnMapperError) + }) + + it(`should throw InvalidColumnMapperError with helpful message mentioning the function name`, () => { + expect(() => { + new ShapeStream({ + url: shapeUrl, + params: { table: `foo` }, + // @ts-expect-error - intentionally testing invalid input + columnMapper: snakeCamelMapper, + }) + }).toThrow(/snakeCamelMapper\(\)/) + }) + + it(`should throw InvalidColumnMapperError when passing createColumnMapper without calling it`, () => { + expect(() => { + new ShapeStream({ + url: shapeUrl, + params: { table: `foo` }, + // @ts-expect-error - intentionally testing invalid input + columnMapper: createColumnMapper, + }) + }).toThrow(InvalidColumnMapperError) + }) + + it(`should throw InvalidColumnMapperError for invalid columnMapper object with helpful guidance`, () => { + expect(() => { + new ShapeStream({ + url: shapeUrl, + params: { table: `foo` }, + // @ts-expect-error - intentionally testing invalid input + columnMapper: { notEncode: () => ``, notDecode: () => `` }, + }) + }).toThrow(/snakeCamelMapper\(\) or createColumnMapper\(\)/) + }) + + it(`should throw InvalidColumnMapperError with fallback message for anonymous functions`, () => { + // Extract function via array to prevent JavaScript from inferring a name + const anonFn = [() => ({ encode: () => ``, decode: () => `` })][0] + expect(() => { + new ShapeStream({ + url: shapeUrl, + params: { table: `foo` }, + // @ts-expect-error - intentionally testing invalid input + columnMapper: anonFn, + }) + }).toThrow(/columnMapper function\(\)/) + }) + + it(`should throw InvalidColumnMapperError for null columnMapper`, () => { + expect(() => { + new ShapeStream({ + url: shapeUrl, + params: { table: `foo` }, + // @ts-expect-error - intentionally testing invalid input + columnMapper: null, + }) + }).toThrow(InvalidColumnMapperError) + }) + + it(`should accept valid columnMapper from snakeCamelMapper()`, () => { + expect(() => { + const stream = new ShapeStream({ + url: shapeUrl, + params: { table: `foo` }, + columnMapper: snakeCamelMapper(), + signal: aborter.signal, + }) + stream.unsubscribeAll() + }).not.toThrow() + }) + + it(`should accept valid columnMapper from createColumnMapper()`, () => { + expect(() => { + const stream = new ShapeStream({ + url: shapeUrl, + params: { table: `foo` }, + columnMapper: createColumnMapper({ user_id: `userId` }), + signal: aborter.signal, + }) + stream.unsubscribeAll() + }).not.toThrow() + }) + + it(`should accept valid custom columnMapper object`, () => { + expect(() => { + const stream = new ShapeStream({ + url: shapeUrl, + params: { table: `foo` }, + // Custom mapper with encode/decode methods + columnMapper: { + encode: (name: string) => name.toLowerCase(), + decode: (name: string) => name.toUpperCase(), + }, + signal: aborter.signal, + }) + stream.unsubscribeAll() + }).not.toThrow() + }) + }) })