From 1acb99eff5257c45fed65caf581c074b99313e84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9tur=20=C3=9E=C3=B3r=20Valdimarsson?= Date: Wed, 18 Mar 2026 23:16:10 +0100 Subject: [PATCH 1/8] Implementing loader strategies Adding feature tests for custom loaders, regression tests and documentation. --- docs/.vitepress/config.mts | 4 + docs/src/api.md | 88 ++-- docs/src/getting-started.md | 1 + docs/src/migration-loading-strategies.md | 109 +++++ src/migrationLoader.ts | 399 ++++++++++++++++++ src/runner.ts | 19 +- test/jiti/jiti.spec.ts | 3 +- test/migrationLoader.spec.ts | 132 ++++++ test/runner.loadMigrations.regression.spec.ts | 223 ++++++++++ 9 files changed, 938 insertions(+), 40 deletions(-) create mode 100644 docs/src/migration-loading-strategies.md create mode 100644 src/migrationLoader.ts create mode 100644 test/migrationLoader.spec.ts create mode 100644 test/runner.loadMigrations.regression.spec.ts diff --git a/docs/.vitepress/config.mts b/docs/.vitepress/config.mts index 283346233..9a34bbb1d 100644 --- a/docs/.vitepress/config.mts +++ b/docs/.vitepress/config.mts @@ -94,6 +94,10 @@ function sidebarReference(): DefaultTheme.SidebarItem[] { text: 'Programmatic API', link: 'api', }, + { + text: 'Migration Loading Strategies', + link: 'migration-loading-strategies', + }, ]; } diff --git a/docs/src/api.md b/docs/src/api.md index 44999ea97..aa0f12922 100644 --- a/docs/src/api.md +++ b/docs/src/api.md @@ -8,29 +8,65 @@ which takes options argument with the following structure (similar to [command l > [!NOTE] > If you use `dbClient`, you should not use `databaseUrl` at the same time and vice versa. -| Option | Type | Description | -| ------------------------ | ------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `databaseUrl` | `string or object` | Connection string or client config which is passed to [new pg.Client](https://node-postgres.com/api/client#constructor) | -| `dbClient` | `pg.Client` | Instance of [new pg.Client](https://node-postgres.com/api/client). Instance should be connected to DB, and after finishing migration, user is responsible to close connection | -| `migrationsTable` | `string` | The table storing which migrations have been run | -| `migrationsSchema` | `string` | The schema storing table which migrations have been run (defaults to same value as `schema`) | -| `schema` | `string or array[string]` | The schema on which migration will be run (defaults to `public`) | -| `dir` | `string or array[string]` | The directory containing your migration files. This path is resolved from `cwd()`. Alternatively, provide a [glob](https://www.npmjs.com/package/glob) pattern or an array of glob patterns and set `useGlob = true`. Note: enabling glob will read both, `dir` _and_ `ignorePattern` as glob patterns | -| `useGlob` | `boolean` | Use [glob](https://www.npmjs.com/package/glob) to find migration files. This will use `dir` _and_ `ignorePattern` to glob-search for migration files. Note: enabling glob will read both, `dir` _and_ `ignorePattern` as glob patterns | -| `checkOrder` | `boolean` | Check order of migrations before running them | -| `direction` | `enum` | `up` or `down` | -| `count` | `number` | Amount of migration to run | -| `timestamp` | `boolean` | Treats `count` as timestamp | -| `ignorePattern` | `string or array[string]` | Regex pattern for file names to ignore (ignores files starting with `.` by default). Alternatively, provide a [glob](https://www.npmjs.com/package/glob) pattern or an array of glob patterns and set `isGlob = true`. Note: enabling glob will read both, `dir` _and_ `ignorePattern` as glob patterns | -| `file` | `string` | Run-only migration with this name | -| `singleTransaction` | `boolean` | Combines all pending migrations into a single transaction so that if any migration fails, all will be rolled back (defaults to `true`) | -| `createSchema` | `boolean` | Creates the configured schema if it doesn't exist | -| `createMigrationsSchema` | `boolean` | Creates the configured migration schema if it doesn't exist | -| `noLock` | `boolean` | Disables locking mechanism and checks | -| `lockValue` | `number` | Value to use for the lock | -| `fake` | `boolean` | Mark migrations as run without actually performing them (use with caution!) | -| `dryRun` | `boolean` | | -| `log` | `function` | Redirect log messages to this function, rather than `console` | -| `logger` | `object with debug/info/warn/error methods` | Redirect messages to this logger object, rather than `console` | -| `verbose` | `boolean` | Print all debug messages like DB queries run (if you switch it on, it will disable `logger.debug` method) | -| `decamelize` | `boolean` | Runs [`decamelize`](https://github.com/salsita/node-pg-migrate/blob/main/src/utils/decamelize.ts) on table/column/etc. names | + +| Option | Type | Description | +| --------------------------- | ------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `databaseUrl` | `string or object` | Connection string or client config which is passed to [new pg.Client](https://node-postgres.com/api/client#constructor) | +| `dbClient` | `pg.Client` | Instance of [new pg.Client](https://node-postgres.com/api/client). Instance should be connected to DB, and after finishing migration, user is responsible to close connection | +| `migrationsTable` | `string` | The table storing which migrations have been run | +| `migrationsSchema` | `string` | The schema storing table which migrations have been run (defaults to same value as `schema`) | +| `schema` | `string or array[string]` | The schema on which migration will be run (defaults to `public`) | +| `dir` | `string or array[string]` | The directory containing your migration files. This path is resolved from `cwd()`. Alternatively, provide a [glob](https://www.npmjs.com/package/glob) pattern or an array of glob patterns and set `useGlob = true`. Note: enabling glob will read both, `dir` *and* `ignorePattern` as glob patterns | +| `useGlob` | `boolean` | Use [glob](https://www.npmjs.com/package/glob) to find migration files. This will use `dir` *and* `ignorePattern` to glob-search for migration files. Note: enabling glob will read both, `dir` *and* `ignorePattern` as glob patterns | +| `checkOrder` | `boolean` | Check order of migrations before running them | +| `direction` | `enum` | `up` or `down` | +| `count` | `number` | Amount of migration to run | +| `timestamp` | `boolean` | Treats `count` as timestamp | +| `ignorePattern` | `string or array[string]` | Regex pattern for file names to ignore (ignores files starting with `.` by default). Alternatively, provide a [glob](https://www.npmjs.com/package/glob) pattern or an array of glob patterns and set `isGlob = true`. Note: enabling glob will read both, `dir` *and* `ignorePattern` as glob patterns | +| `file` | `string` | Run-only migration with this name | +| `singleTransaction` | `boolean` | Combines all pending migrations into a single transaction so that if any migration fails, all will be rolled back (defaults to `true`) | +| `createSchema` | `boolean` | Creates the configured schema if it doesn't exist | +| `createMigrationsSchema` | `boolean` | Creates the configured migration schema if it doesn't exist | +| `noLock` | `boolean` | Disables locking mechanism and checks | +| `lockValue` | `number` | Value to use for the lock | +| `fake` | `boolean` | Mark migrations as run without actually performing them (use with caution!) | +| `dryRun` | `boolean` | | +| `log` | `function` | Redirect log messages to this function, rather than `console` | +| `logger` | `object with debug/info/warn/error methods` | Redirect messages to this logger object, rather than `console` | +| `verbose` | `boolean` | Print all debug messages like DB queries run (if you switch it on, it will disable `logger.debug` method) | +| `decamelize` | `boolean` | Runs `[decamelize](https://github.com/salsita/node-pg-migrate/blob/main/src/utils/decamelize.ts)` on table/column/etc. names | +| `migrationLoaderStrategies` | `MigrationLoaderStrategy[]` | Allows custom loading strategies based on file extensions. If omitted, default behavior is used. See [Migration Loading Strategies](migration-loading-strategies). | + + +### MigrationLoaderStrategy + +``` +export interface MigrationLoaderStrategy { + // File extensions handled by this strategy. + extensions: string[]; + + /** + * Loader that handles conversion of file paths to migration units. + * + * @param filePaths - The file paths to load migrations from. + * @returns The migration units. + */ + loader: MigrationLoader | "default" | "legacySql" | "sql"; +} +``` + +### MigrationUnit + +``` +export type MigrationUnit = { + // The unique identifier for the migration unit. Represents the significant part of the file name used for tracking which migrations have been performed. + id: string; + + // File paths that are part of the migration unit. + filePaths: string[]; + + // The migration builder actions that are contained within the migration files. + actions: MigrationBuilderActions; +}; +``` + diff --git a/docs/src/getting-started.md b/docs/src/getting-started.md index 6a0001c7f..9269f3681 100644 --- a/docs/src/getting-started.md +++ b/docs/src/getting-started.md @@ -202,3 +202,4 @@ Run `npm run migrate up` and there will be a new column in `posts` table :tada: - [CLI commands](/cli) - [Programmatic API](/api) - [Migration files](/migrations/) +- [Migration loading strategies (including SQL)](/migration-loading-strategies) diff --git a/docs/src/migration-loading-strategies.md b/docs/src/migration-loading-strategies.md new file mode 100644 index 000000000..e1b7e6885 --- /dev/null +++ b/docs/src/migration-loading-strategies.md @@ -0,0 +1,109 @@ +# Migration Loading Strategies + +`migrationLoaderStrategies` lets you control how migration files are loaded based on file extension. + +This is useful when you need custom loading behavior, or when you want SQL files to use the new grouped `.up.sql` / `.down.sql` strategy. + +## Default Behavior + +If `migrationLoaderStrategies` is not provided, the loader uses built-in defaults: + +- `.sql` files use the legacy SQL loader (`legacySql`) +- `.js` and `.ts` files use the default loader (`default`) +- unsupported extensions fall back to `default` + +This keeps existing behavior intact. + +## Configuration Shape + +```ts +type MigrationLoader = (filePaths: string[]) => Promise + +interface MigrationLoaderStrategy { + extensions: string[] + loader: MigrationLoader | "default" | "legacySql" | "sql" +} +``` + +## Example: Use Grouped SQL Loader + +This enables grouping `*.up.sql` and `*.down.sql` into one migration unit: + +```ts +import { runner } from "node-pg-migrate" + +await runner({ + databaseUrl: process.env.DATABASE_URL!, + dir: "migrations", + direction: "up", + migrationsTable: "pgmigrations", + migrationLoaderStrategies: [ + { extensions: [".sql"], loader: "sql" }, + ], +}) +``` + +With this configuration: + +- `001_init.up.sql` + `001_init.down.sql` are treated as one migration (`001_init`) +- `001_init.sql` still works as a single-file SQL migration +- mixing `001_init.sql` with `001_init.up.sql` / `001_init.down.sql` throws an error + +## Example: Custom Loader + +You can provide a loader function directly: + +```ts +import type { MigrationLoader } from "node-pg-migrate" +import { runner } from "node-pg-migrate" + +const customLoader: MigrationLoader = async (filePaths) => { + // map files to migration units + return [] +} + +await runner({ + databaseUrl: process.env.DATABASE_URL!, + dir: "migrations", + direction: "up", + migrationsTable: "pgmigrations", + migrationLoaderStrategies: [ + { extensions: [".sql"], loader: "sql" }, + { extensions: [".mjs"], loader: customLoader }, + ], +}) +``` +## Strategy Matching Rules + +- Extension matching is case-insensitive +- Each strategy handles one or more extensions +- If no strategy matches an extension, the `default` loader is used + +# Legacy SQL migrations + +## Why it exists + +The legacy SQL loader has been supported for a long time, even when it was less visible in the docs. + +Common use cases include: + +- onboarding an existing project by importing an initial schema dump as the first migration +- keeping specific advanced migrations as pure SQL when that is cleaner than a builder-based migration + +So if your team already relies on plain `.sql` files, that workflow is still supported. + +## Markers and default fallback + +The classic SQL template uses marker comments: + +```sql +-- Up Migration + +-- Down Migration +``` + +Behavior for a single `.sql` file: + +- when both markers are present, `up` and `down` sections are extracted +- when no markers are present, the full file is treated as an `up` migration +- if there is no `down` section, there is no actionable `down` migration diff --git a/src/migrationLoader.ts b/src/migrationLoader.ts new file mode 100644 index 000000000..63c13c266 --- /dev/null +++ b/src/migrationLoader.ts @@ -0,0 +1,399 @@ +import { createJiti } from "jiti"; +import { type MigrationBuilderActions, sqlMigration } from "./sqlMigration"; +import { basename, extname } from "node:path"; +import { readFile } from "node:fs/promises"; + +/*** + * Migration loader module. + * + * This module is responsible for loading migrations from the file system, allowing for custom loading behaviours. + * If no configuration is provided, loading will be performed in a manner that is identical to the classic loading behaviour. + * + * A new SQL migration loading behaviour is available that supports pairing up/down migrations in separate files using up/down + * suffixes before the .sql extension. + * + * This has been kept intentionally simple for maintainability and readability reasons. + * + */ + + +/** + * The jiti instance to use to load migration files. + */ +export const jiti = createJiti(process.cwd()); + +/************************* + * Types and interfaces + *************************/ + +/** + * A migration unit is a collection of file paths and related migration actions that are related to a single migration. + */ +export type MigrationUnit = { + /** + * The unique identifier for the migration unit. Represents the significant part of the file name. + * Used for tracking which migrations have been performed. + */ + id: string; + + /** + * The file paths that are part of the migration unit. + */ + filePaths: string[]; + + /** + * The migration builder actions that are contained within the migration files. + */ + actions: MigrationBuilderActions; +}; + +/** + * Loader function type. + * + * @param filePaths - The file paths to load migrations from. + * @returns A list of migration units. + */ +export type MigrationLoader = (filePaths: string[]) => Promise; + +/** + * Predefined loader references. + */ +type PredefinedLoader = "default" | "legacySql" | "sql"; + +/** + * Configuration extension to support multiple loader strategies. + */ +export interface MigrationLoaderConfig { + /** + * Configuration for the migration loader strategies. + * Strategies are used to load migrations from different sources based on file patterns. + * + * If no strategy matches, the default strategy is used. + */ + migrationLoaderStrategies?: MigrationLoaderStrategy[]; +} + +/** + * A migration loader strategy is a configuration object that defines a specific loader for given file extensions. + */ +export interface MigrationLoaderStrategy { + /** + * File extensions handled by this strategy. + */ + extensions: string[]; + + /** + * Loader that handles conversion of file paths to migration units. + * Can be a loader function, when used as an API, or a predefined loader when using a json configuration file. + * + * @param filePaths - The file paths to load migrations from. + * @returns The migration units. + */ + loader: MigrationLoader | PredefinedLoader; +} + + +/************************* + * Loader implementations + *************************/ + +/** + * Creates a default migration loader that uses jiti to load migrations from the file paths. + * @returns The default migration loader. + */ +export function createDefaultMigrationLoader(): MigrationLoader { + const loader: MigrationLoader = async (filePaths: string[]) => { + const migrationUnits: MigrationUnit[] = await Promise.all( + filePaths.map(async (filePath) => { + const action: MigrationBuilderActions = await jiti.import(filePath); + return { + id: filePath, + filePaths: [filePath], + actions: action, + }; + }) + ); + return migrationUnits; + }; + return loader; +}; + +/** + * Creates a legacy SQL migration loader that loads migrations from the file paths using the legacy SQL migration loading behaviour. + * @returns The legacy SQL migration loader. + */ +export function createLegacySqlMigrationLoader(): MigrationLoader { + const loader: MigrationLoader = async (filePaths: string[]) => { + const migrationUnits: MigrationUnit[] = await Promise.all( + filePaths.map(async (filePath) => { + const actions = await sqlMigration(filePath); + return { + id: filePath, + filePaths: [filePath], + actions: actions + }; + }) + ); + return migrationUnits; + }; + return loader; +} + +/** + * Creates a SQL migration loader that loads migrations from the file paths using the new SQL migration loading behaviour. + * While is handles the legacy format, it does add new behaviour that may be unwanted in existing usage so it has been separated from the legacy loader and can be used enabled as needed. + * + * @returns The SQL migration loader. + */ +export function createSqlMigrationLoader(): MigrationLoader { + const loader: MigrationLoader = async (filePaths: string[]) => { + const groups = groupSqlFiles(filePaths); + const migrationUnits = await Promise.all(groups.map(async (group) => await readSqlFileGroup(group))); + return migrationUnits; + }; + return loader; +} + + +/************************* + * Loader instances + *************************/ + +/** + * Built-in predefined loaders. + */ +export const builtInLoaders: Record = { + default: createDefaultMigrationLoader(), + legacySql: createLegacySqlMigrationLoader(), + sql: createSqlMigrationLoader(), +}; + +/** + * Default migration loader strategies. + */ +const defaultStrategies: MigrationLoaderStrategy[] = [ + { extensions: [".sql"], loader: builtInLoaders.legacySql }, + { extensions: [".js", ".ts"], loader: builtInLoaders.default }, +]; + +/************************* + * Loader utility functions + *************************/ + +/** + * Resolves the migration loader for a given extension. + * @param config - The Runner configuration object. + * @param extension - The extension to resolve the loader for. + * @returns The migration loader. + */ +function resolveMigrationLoader(config: MigrationLoaderConfig, extension: string): MigrationLoader { + + const normalizedExtension = extension.toLowerCase(); + const strategies = config.migrationLoaderStrategies ?? defaultStrategies; + + const foundStrategy = strategies.find(strategy => + strategy.extensions.some(ext => ext.toLowerCase() === normalizedExtension) + ); + + const loader = foundStrategy?.loader ?? builtInLoaders.default; + + if (typeof loader === "string") { + const resolved = builtInLoaders[loader]; + if (!resolved) { + throw new Error(`Unknown predefined loader: ${loader}`); + } + return resolved; + } + + return loader; +} + +/** + * Associates the file paths to their extensions. + * @param filePaths - The file paths to associate. + * @returns The file paths associated to their extensions. + */ +function associatePathsToExtensions(filePaths: string[]): Map { + const filesByExtension = new Map(); + for (const filePath of filePaths) { + const ext = extname(filePath).toLowerCase(); + if (!filesByExtension.has(ext)) { + filesByExtension.set(ext, []); + } + filesByExtension.get(ext)!.push(filePath); + } + return filesByExtension; +}; + +/*********************************** + * Migration loader main function + ***********************************/ + +/** + * Loads the migration units from the file paths. + * @param config - The migration loader configuration. + * @param filePaths - List of files containing migrations. + * @returns List of migration units, sorted according to the given file paths. + */ +export async function loadMigrationUnits(config: MigrationLoaderConfig, filePaths: string[]): Promise { + const migrationUnits: MigrationUnit[] = []; + const filesByExtension = associatePathsToExtensions(filePaths); + for (const [extension, filePaths] of filesByExtension) { + const loader = resolveMigrationLoader(config, extension); + const units = await loader(filePaths); + migrationUnits.push(...units); + } + // Since the sql migration loader modifies the id, it is no longer comparable. Hence we sort by the file path of the first file in the unit that always exists. + const sortedMigrationUnits = migrationUnits.sort((a, b) => a.filePaths[0]!.localeCompare(b.filePaths[0]!)); + return sortedMigrationUnits; +} + +/***************************************** + * New SQL migration loading behaviour. + *****************************************/ + +// Helper types + +/** + * A parsed SQL file is a file that has been parsed and contains the id, direction and file path. + * An intermediate step before the migration unit is created. + */ +type ParsedSqlFile = { + id: string; + direction: "up" | "down" | "none"; + filePath: string; +}; +/** + * A SQL group is a group of SQL files associated by significant part of the filename. + */ +type SqlGroup = { + id: string; + up?: string; + down?: string; + single?: string; +}; + +/** + * Parses a SQL file and returns the parsed file. + * @param filePath - The file path to parse. + * @returns The parsed file. + */ +function parseSqlFile(filePath: string): ParsedSqlFile { + const name = basename(filePath, ".sql"); + + if (name.endsWith(".up")) { + return { + id: name.slice(0, -3), + direction: "up", + filePath, + }; + } + + if (name.endsWith(".down")) { + return { + id: name.slice(0, -5), + direction: "down", + filePath, + }; + } + + return { + id: name, + direction: "none", + filePath, + }; +} + +/** + * Groups the SQL files by their significant part of the filename. + * @param filePaths - The file paths to group. + * @returns An array of SQL groups. + * + * Throws an error if the files are not properly paired. + * + */ +function groupSqlFiles(filePaths: string[]): SqlGroup[] { + const groups = new Map(); + for (const filePath of filePaths) { + const parsed = parseSqlFile(filePath); + + if (!groups.has(parsed.id)) { + groups.set(parsed.id, {id: parsed.id}); + } + + const group = groups.get(parsed.id)!; + + if (parsed.direction === "up") { + if (group.up) throw new Error(`Duplicate .up.sql for ${parsed.id}`); + group.up = parsed.filePath; + } else if (parsed.direction === "down") { + if (group.down) throw new Error(`Duplicate .down.sql for ${parsed.id}`); + group.down = parsed.filePath; + } else { + if (group.single) throw new Error(`Duplicate .sql for ${parsed.id}`); + group.single = parsed.filePath; + } + } + for (const [id, group] of groups) { + if (group.single && (group.up || group.down)) { + throw new Error( + `Conflicting SQL migration files for ${id}: cannot mix .sql with .up/.down` + ); + } + + if (group.down && !group.up) { + throw new Error( + `Found .down.sql without matching .up.sql for ${id}` + ); + } + } + return Array.from(groups.values()); +} + +function sqlGroupId(group: SqlGroup): string { + let filePath = group.single ?? group.up ?? group.down; + if (!filePath) { + throw new Error(`No SQL file found for group ${group.id}`); + } + return filePath.replace(/\.up\.sql$/, ".sql") + .replace(/\.down\.sql$/, ".sql"); +} + +/** + * Performs the actual reading of a SQL file group and returns the migration unit. + * @param group - The SQL file group to read. + * @returns The migration unit. + */ +async function readSqlFileGroup(group: SqlGroup): Promise { + let actions: MigrationBuilderActions + + if (group.single) { + actions = await sqlMigration(group.single); + } else { + + if (!group.up) { + // Since a down migration without an up migration is a deviation from expected behaviour, we throw an error. + throw new Error(`Missing .up.sql for ${group.id}`); + } + const upSql = await readFile(group.up, "utf-8"); + + const downSql = group.down ? await readFile(group.down, "utf-8") : undefined; + actions = { + up: (pgm) => pgm.sql(upSql), + down: downSql ? (pgm) => pgm.sql(downSql) : undefined, + shorthands: {}, + }; + } + + const filePaths = [ + group.up, + group.down, + group.single, + ].filter((p): p is string => Boolean(p)); + + return { + id: sqlGroupId(group), + filePaths: filePaths, + actions: actions, + }; +} \ No newline at end of file diff --git a/src/runner.ts b/src/runner.ts index c78fc1420..84fb3f2b3 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -1,5 +1,3 @@ -import { createJiti } from 'jiti'; -import { extname } from 'node:path'; import type { ClientBase, ClientConfig } from 'pg'; import type { DBConnection } from './db'; import { db as Db } from './db'; @@ -7,9 +5,8 @@ import type { LogFn, Logger } from './logger'; import type { RunMigration } from './migration'; import { getMigrationFilePaths, Migration } from './migration'; import type { ColumnDefinitions } from './operations/tables'; -import type { MigrationBuilderActions } from './sqlMigration'; -import { sqlMigration as migrateSqlFile } from './sqlMigration'; import { createSchemalize, getMigrationTableSchema, getSchemas } from './utils'; +import { loadMigrationUnits, type MigrationLoaderConfig } from './migrationLoader'; export interface RunnerOptionConfig { /** @@ -155,7 +152,7 @@ export interface RunnerOptionClient { dbClient: ClientBase; } -export type RunnerOption = RunnerOptionConfig & +export type RunnerOption = RunnerOptionConfig & MigrationLoaderConfig & (RunnerOptionClient | RunnerOptionUrl); /** @@ -167,8 +164,6 @@ const idColumn = 'id'; const nameColumn = 'name'; const runOnColumn = 'run_on'; -export const jiti = createJiti(process.cwd()); - export async function loadMigrations( db: DBConnection, options: RunnerOption, @@ -181,13 +176,11 @@ export async function loadMigrations( useGlob: options.useGlob, logger, }); - const migrations: Migration[] = []; - for (const filePath of absoluteFilePaths) { - const actions: MigrationBuilderActions = - extname(filePath) === '.sql' - ? await migrateSqlFile(filePath) - : await jiti.import(filePath); + + // Actual loading of files has been delegated to the loadMigrationUnits function. + const migrationUnits = await loadMigrationUnits(options, absoluteFilePaths); + for (const { id: filePath, actions } of migrationUnits) { shorthands = { ...shorthands, ...actions.shorthands }; migrations.push( diff --git a/test/jiti/jiti.spec.ts b/test/jiti/jiti.spec.ts index e55ec9c0c..1c03c8bc9 100644 --- a/test/jiti/jiti.spec.ts +++ b/test/jiti/jiti.spec.ts @@ -3,7 +3,8 @@ import { join, resolve } from 'node:path'; import { describe, expect, it, vi } from 'vitest'; import type { Migration, RunnerOption } from '../../src'; import type { DBConnection } from '../../src/db'; -import { jiti, loadMigrations } from '../../src/runner'; +import { loadMigrations } from '../../src/runner'; +import { jiti } from '../../src/migrationLoader'; describe('loadMigrations', () => { it('should load migration files using jiti', async () => { diff --git a/test/migrationLoader.spec.ts b/test/migrationLoader.spec.ts new file mode 100644 index 000000000..d1547bba8 --- /dev/null +++ b/test/migrationLoader.spec.ts @@ -0,0 +1,132 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { describe, expect, it } from "vitest"; +import { + loadMigrationUnits, + type MigrationLoader, + type MigrationLoaderConfig, + type MigrationUnit, +} from "../src/migrationLoader"; + +async function withTempDir(run: (dir: string) => Promise): Promise { + const dir = await mkdtemp(join(tmpdir(), "npm-migration-loader-test-")); + try { + return await run(dir); + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + +async function writeMigrationFile( + dir: string, + fileName: string, + content: string +): Promise { + const path = join(dir, fileName); + await writeFile(path, content, "utf8"); + return path; +} + +describe("loadMigrationUnits", () => { + it("keeps legacy SQL behavior by default (.up/.down are separate migrations)", async () => { + await withTempDir(async (dir) => { + const upPath = await writeMigrationFile( + dir, + "001_create_users.up.sql", + "-- up migration\nCREATE TABLE users(id serial primary key);\n" + ); + const downPath = await writeMigrationFile( + dir, + "001_create_users.down.sql", + "-- down migration\nDROP TABLE users;\n" + ); + + const units = await loadMigrationUnits({}, [upPath, downPath]); + + expect(units).toHaveLength(2); + expect(units.map((u) => u.id)).toEqual([downPath, upPath].sort()); + expect(units.every((u) => u.filePaths.length === 1)).toBe(true); + }); + }); + + it("groups .up/.down SQL files when the new sql loader is configured", async () => { + await withTempDir(async (dir) => { + const upPath = await writeMigrationFile( + dir, + "001_create_users.up.sql", + "CREATE TABLE users(id serial primary key);\n" + ); + const downPath = await writeMigrationFile( + dir, + "001_create_users.down.sql", + "DROP TABLE users;\n" + ); + + const config: MigrationLoaderConfig = { + migrationLoaderStrategies: [{ extensions: [".sql"], loader: "sql" }], + }; + + const units = await loadMigrationUnits(config, [upPath, downPath]); + + expect(units).toHaveLength(1); + expect(units[0].id).toBe(join(dir, "001_create_users.sql")); + expect(units[0].filePaths).toEqual([upPath, downPath]); + expect(units[0].actions.up).toBeTypeOf("function"); + expect(units[0].actions.down).toBeTypeOf("function"); + }); + }); + + it("throws when sql loader sees .down.sql without matching .up.sql", async () => { + await withTempDir(async (dir) => { + const downPath = await writeMigrationFile( + dir, + "001_create_users.down.sql", + "DROP TABLE users;\n" + ); + + const config: MigrationLoaderConfig = { + migrationLoaderStrategies: [{ extensions: [".sql"], loader: "sql" }], + }; + + await expect(loadMigrationUnits(config, [downPath])).rejects.toThrow( + "Found .down.sql without matching .up.sql for 001_create_users" + ); + }); + }); + + it("uses custom loader only for matched extension buckets", async () => { + await withTempDir(async (dir) => { + const jsPath = await writeMigrationFile( + dir, + "001_script.js", + "export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n" + ); + const mjsPath = await writeMigrationFile( + dir, + "002_script.mjs", + "export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n" + ); + + const customUnits: MigrationUnit[] = [ + { + id: "custom-js-id", + filePaths: [jsPath], + actions: { up: () => undefined, down: () => undefined, shorthands: {} }, + }, + ]; + const customLoader: MigrationLoader = async () => customUnits; + + const config: MigrationLoaderConfig = { + migrationLoaderStrategies: [{ extensions: [".js"], loader: customLoader }], + }; + + const units = await loadMigrationUnits(config, [jsPath, mjsPath]); + const ids = units.map((u) => u.id); + + expect(ids).toContain("custom-js-id"); + expect(ids).toContain(mjsPath); + expect(units).toHaveLength(2); + }); + }); +}); diff --git a/test/runner.loadMigrations.regression.spec.ts b/test/runner.loadMigrations.regression.spec.ts new file mode 100644 index 000000000..8bd5e01ec --- /dev/null +++ b/test/runner.loadMigrations.regression.spec.ts @@ -0,0 +1,223 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { describe, expect, it } from "vitest"; +import type { RunnerOption } from "../src"; +import type { DBConnection } from "../src/db"; +import { loadMigrations } from "../src/runner"; + +async function withTempDir(run: (dir: string) => Promise): Promise { + const dir = await mkdtemp(join(tmpdir(), "npm-load-migrations-regression-")); + try { + return await run(dir); + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + +async function writeMigrationFile( + dir: string, + fileName: string, + content: string +): Promise { + const path = join(dir, fileName); + await writeFile(path, content, "utf8"); + return path; +} + +describe("loadMigrations regression behavior", () => { + it("keeps legacy .sql handling and preserves migration paths by default", async () => { + await withTempDir(async (dir) => { + const singleSqlPath = await writeMigrationFile( + dir, + "001_single.sql", + "-- up migration\nSELECT 1;\n-- down migration\nSELECT 0;\n" + ); + const downSqlPath = await writeMigrationFile( + dir, + "002_pair.down.sql", + "-- down migration\nDROP TABLE users;\n" + ); + const upSqlPath = await writeMigrationFile( + dir, + "002_pair.up.sql", + "-- up migration\nCREATE TABLE users(id serial primary key);\n" + ); + const jsPath = await writeMigrationFile( + dir, + "003_script.js", + "export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n" + ); + + const db: DBConnection = {} as never; + const options: RunnerOption = { + dir, + ignorePattern: undefined, + useGlob: false, + migrationsTable: "migrations", + direction: "up", + databaseUrl: "postgres://user:pass@localhost/db", + }; + + const migrations = await loadMigrations(db, options, console); + + // Default behavior should keep .up/.down.sql as separate migrations. + expect(migrations).toHaveLength(4); + + // Paths should remain concrete source paths (no id normalization by default). + expect(migrations.map((migration) => migration.path)).toEqual([ + singleSqlPath, + downSqlPath, + upSqlPath, + jsPath, + ]); + + // Names are distinct for .up/.down pair under legacy default SQL handling. + expect(migrations.map((migration) => migration.name)).toEqual([ + "001_single", + "002_pair.down", + "002_pair.up", + "003_script", + ]); + }); + }); + + it("keeps init before next when 001_init.sql is split into 001_init.up/down.sql", async () => { + await withTempDir(async (dir) => { + const initUpPath = await writeMigrationFile( + dir, + "001_init.up.sql", + "CREATE TABLE init_table(id serial primary key);\n" + ); + const initDownPath = await writeMigrationFile( + dir, + "001_init.down.sql", + "DROP TABLE init_table;\n" + ); + const nextPath = await writeMigrationFile( + dir, + "002_next.sql", + "-- up migration\nSELECT 2;\n" + ); + + const db: DBConnection = {} as never; + const options: RunnerOption = { + dir, + ignorePattern: undefined, + useGlob: false, + migrationsTable: "migrations", + direction: "up", + databaseUrl: "postgres://user:pass@localhost/db", + migrationLoaderStrategies: [{ extensions: [".sql"], loader: "sql" }], + }; + + const migrations = await loadMigrations(db, options, console); + + expect(migrations).toHaveLength(2); + expect(migrations.map((migration) => migration.path)).toEqual([ + join(dir, "001_init.sql"), + nextPath, + ]); + expect(migrations.map((migration) => migration.name)).toEqual([ + "001_init", + "002_next", + ]); + + // Ensure files are truly split and still represented as one ordered migration. + expect(initUpPath).toContain(".up.sql"); + expect(initDownPath).toContain(".down.sql"); + }); + }); + + it("falls back to default loader for unmatched extensions when only .sql strategy is configured", async () => { + await withTempDir(async (dir) => { + const sqlPath = await writeMigrationFile( + dir, + "001_init.sql", + "-- up migration\nSELECT 1;\n" + ); + const jsPath = await writeMigrationFile( + dir, + "002_next.js", + "export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n" + ); + + const db: DBConnection = {} as never; + const options: RunnerOption = { + dir, + ignorePattern: undefined, + useGlob: false, + migrationsTable: "migrations", + direction: "up", + databaseUrl: "postgres://user:pass@localhost/db", + migrationLoaderStrategies: [{ extensions: [".sql"], loader: "sql" }], + }; + + const migrations = await loadMigrations(db, options, console); + expect(migrations.map((migration) => migration.path)).toEqual([ + sqlPath, + jsPath, + ]); + expect(migrations.map((migration) => migration.name)).toEqual([ + "001_init", + "002_next", + ]); + }); + }); + + it("throws when .sql and .up/.down forms are mixed for the same migration id", async () => { + await withTempDir(async (dir) => { + await writeMigrationFile( + dir, + "001_init.sql", + "-- up migration\nSELECT 1;\n" + ); + await writeMigrationFile( + dir, + "001_init.up.sql", + "CREATE TABLE t(id serial primary key);\n" + ); + + const db: DBConnection = {} as never; + const options: RunnerOption = { + dir, + ignorePattern: undefined, + useGlob: false, + migrationsTable: "migrations", + direction: "up", + databaseUrl: "postgres://user:pass@localhost/db", + migrationLoaderStrategies: [{ extensions: [".sql"], loader: "sql" }], + }; + + await expect(loadMigrations(db, options, console)).rejects.toThrow( + "Conflicting SQL migration files for 001_init: cannot mix .sql with .up/.down" + ); + }); + }); + + it("matches strategy extensions case-insensitively", async () => { + await withTempDir(async (dir) => { + const jsPath = await writeMigrationFile( + dir, + "001_case.js", + "export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n" + ); + + const db: DBConnection = {} as never; + const options: RunnerOption = { + dir, + ignorePattern: undefined, + useGlob: false, + migrationsTable: "migrations", + direction: "up", + databaseUrl: "postgres://user:pass@localhost/db", + migrationLoaderStrategies: [{ extensions: [".JS"], loader: "default" }], + }; + + const migrations = await loadMigrations(db, options, console); + expect(migrations).toHaveLength(1); + expect(migrations[0].path).toBe(jsPath); + expect(migrations[0].name).toBe("001_case"); + }); + }); +}); From 51fcdd68fa2212c15977e8b2d3d1f023919987a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9tur=20=C3=9E=C3=B3r=20Valdimarsson?= Date: Thu, 19 Mar 2026 18:08:35 +0100 Subject: [PATCH 2/8] Fixes for linting errors. Adjustment of import of RunnerOption in node-pg-migrate.ts to avoid ambiguity, harmonizing the import with that of runner. --- bin/node-pg-migrate.ts | 2 +- docs/src/api.md | 57 ++++--- docs/src/migration-loading-strategies.md | 41 +++-- src/migrationLoader.ts | 151 ++++++++++-------- src/runner.ts | 8 +- test/jiti/jiti.spec.ts | 2 +- test/migrationLoader.spec.ts | 93 ++++++----- test/runner.loadMigrations.regression.spec.ts | 142 ++++++++-------- 8 files changed, 259 insertions(+), 237 deletions(-) diff --git a/bin/node-pg-migrate.ts b/bin/node-pg-migrate.ts index e5f19b213..cd3e7aace 100755 --- a/bin/node-pg-migrate.ts +++ b/bin/node-pg-migrate.ts @@ -5,6 +5,7 @@ import type { DotenvConfigOptions } from 'dotenv'; // otherwise this could not be imported by esm // @ts-ignore: when a clean was made, the types are not present in the first run import { createJiti } from 'jiti'; +import type { RunnerOption } from 'node-pg-migrate'; import { Migration, PG_MIGRATE_LOCK_ID, @@ -19,7 +20,6 @@ import type ConnectionParametersType from 'pg/lib/connection-parameters'; // @ts-expect-error type exports from @types/pg doesn't match importing import ConnectionParameters from 'pg/lib/connection-parameters.js'; import yargs from 'yargs/yargs'; -import type { RunnerOption } from '../src'; import type { FilenameFormat } from '../src/migration'; process.on('uncaughtException', (err) => { diff --git a/docs/src/api.md b/docs/src/api.md index aa0f12922..395d835f6 100644 --- a/docs/src/api.md +++ b/docs/src/api.md @@ -8,35 +8,33 @@ which takes options argument with the following structure (similar to [command l > [!NOTE] > If you use `dbClient`, you should not use `databaseUrl` at the same time and vice versa. - -| Option | Type | Description | -| --------------------------- | ------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `databaseUrl` | `string or object` | Connection string or client config which is passed to [new pg.Client](https://node-postgres.com/api/client#constructor) | -| `dbClient` | `pg.Client` | Instance of [new pg.Client](https://node-postgres.com/api/client). Instance should be connected to DB, and after finishing migration, user is responsible to close connection | -| `migrationsTable` | `string` | The table storing which migrations have been run | -| `migrationsSchema` | `string` | The schema storing table which migrations have been run (defaults to same value as `schema`) | -| `schema` | `string or array[string]` | The schema on which migration will be run (defaults to `public`) | -| `dir` | `string or array[string]` | The directory containing your migration files. This path is resolved from `cwd()`. Alternatively, provide a [glob](https://www.npmjs.com/package/glob) pattern or an array of glob patterns and set `useGlob = true`. Note: enabling glob will read both, `dir` *and* `ignorePattern` as glob patterns | -| `useGlob` | `boolean` | Use [glob](https://www.npmjs.com/package/glob) to find migration files. This will use `dir` *and* `ignorePattern` to glob-search for migration files. Note: enabling glob will read both, `dir` *and* `ignorePattern` as glob patterns | -| `checkOrder` | `boolean` | Check order of migrations before running them | -| `direction` | `enum` | `up` or `down` | -| `count` | `number` | Amount of migration to run | -| `timestamp` | `boolean` | Treats `count` as timestamp | -| `ignorePattern` | `string or array[string]` | Regex pattern for file names to ignore (ignores files starting with `.` by default). Alternatively, provide a [glob](https://www.npmjs.com/package/glob) pattern or an array of glob patterns and set `isGlob = true`. Note: enabling glob will read both, `dir` *and* `ignorePattern` as glob patterns | -| `file` | `string` | Run-only migration with this name | -| `singleTransaction` | `boolean` | Combines all pending migrations into a single transaction so that if any migration fails, all will be rolled back (defaults to `true`) | -| `createSchema` | `boolean` | Creates the configured schema if it doesn't exist | -| `createMigrationsSchema` | `boolean` | Creates the configured migration schema if it doesn't exist | -| `noLock` | `boolean` | Disables locking mechanism and checks | -| `lockValue` | `number` | Value to use for the lock | -| `fake` | `boolean` | Mark migrations as run without actually performing them (use with caution!) | -| `dryRun` | `boolean` | | -| `log` | `function` | Redirect log messages to this function, rather than `console` | -| `logger` | `object with debug/info/warn/error methods` | Redirect messages to this logger object, rather than `console` | -| `verbose` | `boolean` | Print all debug messages like DB queries run (if you switch it on, it will disable `logger.debug` method) | -| `decamelize` | `boolean` | Runs `[decamelize](https://github.com/salsita/node-pg-migrate/blob/main/src/utils/decamelize.ts)` on table/column/etc. names | -| `migrationLoaderStrategies` | `MigrationLoaderStrategy[]` | Allows custom loading strategies based on file extensions. If omitted, default behavior is used. See [Migration Loading Strategies](migration-loading-strategies). | - +| Option | Type | Description | +| --------------------------- | ------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `databaseUrl` | `string or object` | Connection string or client config which is passed to [new pg.Client](https://node-postgres.com/api/client#constructor) | +| `dbClient` | `pg.Client` | Instance of [new pg.Client](https://node-postgres.com/api/client). Instance should be connected to DB, and after finishing migration, user is responsible to close connection | +| `migrationsTable` | `string` | The table storing which migrations have been run | +| `migrationsSchema` | `string` | The schema storing table which migrations have been run (defaults to same value as `schema`) | +| `schema` | `string or array[string]` | The schema on which migration will be run (defaults to `public`) | +| `dir` | `string or array[string]` | The directory containing your migration files. This path is resolved from `cwd()`. Alternatively, provide a [glob](https://www.npmjs.com/package/glob) pattern or an array of glob patterns and set `useGlob = true`. Note: enabling glob will read both, `dir` _and_ `ignorePattern` as glob patterns | +| `useGlob` | `boolean` | Use [glob](https://www.npmjs.com/package/glob) to find migration files. This will use `dir` _and_ `ignorePattern` to glob-search for migration files. Note: enabling glob will read both, `dir` _and_ `ignorePattern` as glob patterns | +| `checkOrder` | `boolean` | Check order of migrations before running them | +| `direction` | `enum` | `up` or `down` | +| `count` | `number` | Amount of migration to run | +| `timestamp` | `boolean` | Treats `count` as timestamp | +| `ignorePattern` | `string or array[string]` | Regex pattern for file names to ignore (ignores files starting with `.` by default). Alternatively, provide a [glob](https://www.npmjs.com/package/glob) pattern or an array of glob patterns and set `isGlob = true`. Note: enabling glob will read both, `dir` _and_ `ignorePattern` as glob patterns | +| `file` | `string` | Run-only migration with this name | +| `singleTransaction` | `boolean` | Combines all pending migrations into a single transaction so that if any migration fails, all will be rolled back (defaults to `true`) | +| `createSchema` | `boolean` | Creates the configured schema if it doesn't exist | +| `createMigrationsSchema` | `boolean` | Creates the configured migration schema if it doesn't exist | +| `noLock` | `boolean` | Disables locking mechanism and checks | +| `lockValue` | `number` | Value to use for the lock | +| `fake` | `boolean` | Mark migrations as run without actually performing them (use with caution!) | +| `dryRun` | `boolean` | | +| `log` | `function` | Redirect log messages to this function, rather than `console` | +| `logger` | `object with debug/info/warn/error methods` | Redirect messages to this logger object, rather than `console` | +| `verbose` | `boolean` | Print all debug messages like DB queries run (if you switch it on, it will disable `logger.debug` method) | +| `decamelize` | `boolean` | Runs `[decamelize](https://github.com/salsita/node-pg-migrate/blob/main/src/utils/decamelize.ts)` on table/column/etc. names | +| `migrationLoaderStrategies` | `MigrationLoaderStrategy[]` | Allows custom loading strategies based on file extensions. If omitted, default behavior is used. See [Migration Loading Strategies](migration-loading-strategies). | ### MigrationLoaderStrategy @@ -69,4 +67,3 @@ export type MigrationUnit = { actions: MigrationBuilderActions; }; ``` - diff --git a/docs/src/migration-loading-strategies.md b/docs/src/migration-loading-strategies.md index e1b7e6885..10c9df6f9 100644 --- a/docs/src/migration-loading-strategies.md +++ b/docs/src/migration-loading-strategies.md @@ -17,11 +17,11 @@ This keeps existing behavior intact. ## Configuration Shape ```ts -type MigrationLoader = (filePaths: string[]) => Promise +type MigrationLoader = (filePaths: string[]) => Promise; interface MigrationLoaderStrategy { - extensions: string[] - loader: MigrationLoader | "default" | "legacySql" | "sql" + extensions: string[]; + loader: MigrationLoader | 'default' | 'legacySql' | 'sql'; } ``` @@ -30,17 +30,15 @@ interface MigrationLoaderStrategy { This enables grouping `*.up.sql` and `*.down.sql` into one migration unit: ```ts -import { runner } from "node-pg-migrate" +import { runner } from 'node-pg-migrate'; await runner({ databaseUrl: process.env.DATABASE_URL!, - dir: "migrations", - direction: "up", - migrationsTable: "pgmigrations", - migrationLoaderStrategies: [ - { extensions: [".sql"], loader: "sql" }, - ], -}) + dir: 'migrations', + direction: 'up', + migrationsTable: 'pgmigrations', + migrationLoaderStrategies: [{ extensions: ['.sql'], loader: 'sql' }], +}); ``` With this configuration: @@ -54,25 +52,26 @@ With this configuration: You can provide a loader function directly: ```ts -import type { MigrationLoader } from "node-pg-migrate" -import { runner } from "node-pg-migrate" +import type { MigrationLoader } from 'node-pg-migrate'; +import { runner } from 'node-pg-migrate'; const customLoader: MigrationLoader = async (filePaths) => { // map files to migration units - return [] -} + return []; +}; await runner({ databaseUrl: process.env.DATABASE_URL!, - dir: "migrations", - direction: "up", - migrationsTable: "pgmigrations", + dir: 'migrations', + direction: 'up', + migrationsTable: 'pgmigrations', migrationLoaderStrategies: [ - { extensions: [".sql"], loader: "sql" }, - { extensions: [".mjs"], loader: customLoader }, + { extensions: ['.sql'], loader: 'sql' }, + { extensions: ['.mjs'], loader: customLoader }, ], -}) +}); ``` + ## Strategy Matching Rules - Extension matching is case-insensitive diff --git a/src/migrationLoader.ts b/src/migrationLoader.ts index 63c13c266..5d491706a 100644 --- a/src/migrationLoader.ts +++ b/src/migrationLoader.ts @@ -1,22 +1,22 @@ -import { createJiti } from "jiti"; -import { type MigrationBuilderActions, sqlMigration } from "./sqlMigration"; -import { basename, extname } from "node:path"; -import { readFile } from "node:fs/promises"; +import { createJiti } from 'jiti'; +import { readFile } from 'node:fs/promises'; +import { basename, extname } from 'node:path'; +import type { MigrationBuilderActions } from './sqlMigration'; +import { sqlMigration } from './sqlMigration'; /*** * Migration loader module. - * + * * This module is responsible for loading migrations from the file system, allowing for custom loading behaviours. * If no configuration is provided, loading will be performed in a manner that is identical to the classic loading behaviour. - * - * A new SQL migration loading behaviour is available that supports pairing up/down migrations in separate files using up/down + * + * A new SQL migration loading behaviour is available that supports pairing up/down migrations in separate files using up/down * suffixes before the .sql extension. - * + * * This has been kept intentionally simple for maintainability and readability reasons. - * + * */ - /** * The jiti instance to use to load migration files. */ @@ -58,7 +58,7 @@ export type MigrationLoader = (filePaths: string[]) => Promise; /** * Predefined loader references. */ -type PredefinedLoader = "default" | "legacySql" | "sql"; +type PredefinedLoader = 'default' | 'legacySql' | 'sql'; /** * Configuration extension to support multiple loader strategies. @@ -92,7 +92,6 @@ export interface MigrationLoaderStrategy { loader: MigrationLoader | PredefinedLoader; } - /************************* * Loader implementations *************************/ @@ -115,8 +114,9 @@ export function createDefaultMigrationLoader(): MigrationLoader { ); return migrationUnits; }; + return loader; -}; +} /** * Creates a legacy SQL migration loader that loads migrations from the file paths using the legacy SQL migration loading behaviour. @@ -130,31 +130,34 @@ export function createLegacySqlMigrationLoader(): MigrationLoader { return { id: filePath, filePaths: [filePath], - actions: actions + actions: actions, }; }) ); return migrationUnits; }; + return loader; } /** * Creates a SQL migration loader that loads migrations from the file paths using the new SQL migration loading behaviour. - * While is handles the legacy format, it does add new behaviour that may be unwanted in existing usage so it has been separated from the legacy loader and can be used enabled as needed. - * + * While is handles the legacy format, it does add new behaviour that may be unwanted in existing usage so it has been separated from the legacy loader and can be used enabled as needed. + * * @returns The SQL migration loader. */ export function createSqlMigrationLoader(): MigrationLoader { const loader: MigrationLoader = async (filePaths: string[]) => { const groups = groupSqlFiles(filePaths); - const migrationUnits = await Promise.all(groups.map(async (group) => await readSqlFileGroup(group))); + const migrationUnits = await Promise.all( + groups.map(async (group) => await readSqlFileGroup(group)) + ); return migrationUnits; }; + return loader; } - /************************* * Loader instances *************************/ @@ -172,8 +175,8 @@ export const builtInLoaders: Record = { * Default migration loader strategies. */ const defaultStrategies: MigrationLoaderStrategy[] = [ - { extensions: [".sql"], loader: builtInLoaders.legacySql }, - { extensions: [".js", ".ts"], loader: builtInLoaders.default }, + { extensions: ['.sql'], loader: builtInLoaders.legacySql }, + { extensions: ['.js', '.ts'], loader: builtInLoaders.default }, ]; /************************* @@ -186,22 +189,25 @@ const defaultStrategies: MigrationLoaderStrategy[] = [ * @param extension - The extension to resolve the loader for. * @returns The migration loader. */ -function resolveMigrationLoader(config: MigrationLoaderConfig, extension: string): MigrationLoader { - +function resolveMigrationLoader( + config: MigrationLoaderConfig, + extension: string +): MigrationLoader { const normalizedExtension = extension.toLowerCase(); const strategies = config.migrationLoaderStrategies ?? defaultStrategies; - const foundStrategy = strategies.find(strategy => - strategy.extensions.some(ext => ext.toLowerCase() === normalizedExtension) + const foundStrategy = strategies.find((strategy) => + strategy.extensions.some((ext) => ext.toLowerCase() === normalizedExtension) ); const loader = foundStrategy?.loader ?? builtInLoaders.default; - if (typeof loader === "string") { + if (typeof loader === 'string') { const resolved = builtInLoaders[loader]; if (!resolved) { throw new Error(`Unknown predefined loader: ${loader}`); } + return resolved; } @@ -213,17 +219,21 @@ function resolveMigrationLoader(config: MigrationLoaderConfig, extension: string * @param filePaths - The file paths to associate. * @returns The file paths associated to their extensions. */ -function associatePathsToExtensions(filePaths: string[]): Map { +function associatePathsToExtensions( + filePaths: string[] +): Map { const filesByExtension = new Map(); - for (const filePath of filePaths) { - const ext = extname(filePath).toLowerCase(); - if (!filesByExtension.has(ext)) { - filesByExtension.set(ext, []); - } - filesByExtension.get(ext)!.push(filePath); + for (const filePath of filePaths) { + const ext = extname(filePath).toLowerCase(); + if (!filesByExtension.has(ext)) { + filesByExtension.set(ext, []); } + + filesByExtension.get(ext)?.push(filePath); + } + return filesByExtension; -}; +} /*********************************** * Migration loader main function @@ -235,7 +245,10 @@ function associatePathsToExtensions(filePaths: string[]): Map * @param filePaths - List of files containing migrations. * @returns List of migration units, sorted according to the given file paths. */ -export async function loadMigrationUnits(config: MigrationLoaderConfig, filePaths: string[]): Promise { +export async function loadMigrationUnits( + config: MigrationLoaderConfig, + filePaths: string[] +): Promise { const migrationUnits: MigrationUnit[] = []; const filesByExtension = associatePathsToExtensions(filePaths); for (const [extension, filePaths] of filesByExtension) { @@ -243,15 +256,18 @@ export async function loadMigrationUnits(config: MigrationLoaderConfig, filePath const units = await loader(filePaths); migrationUnits.push(...units); } + // Since the sql migration loader modifies the id, it is no longer comparable. Hence we sort by the file path of the first file in the unit that always exists. - const sortedMigrationUnits = migrationUnits.sort((a, b) => a.filePaths[0]!.localeCompare(b.filePaths[0]!)); + const sortedMigrationUnits = migrationUnits.toSorted((a, b) => + a.filePaths[0].localeCompare(b.filePaths[0]) + ); return sortedMigrationUnits; } /***************************************** * New SQL migration loading behaviour. *****************************************/ - + // Helper types /** @@ -260,7 +276,7 @@ export async function loadMigrationUnits(config: MigrationLoaderConfig, filePath */ type ParsedSqlFile = { id: string; - direction: "up" | "down" | "none"; + direction: 'up' | 'down' | 'none'; filePath: string; }; /** @@ -279,27 +295,27 @@ type SqlGroup = { * @returns The parsed file. */ function parseSqlFile(filePath: string): ParsedSqlFile { - const name = basename(filePath, ".sql"); + const name = basename(filePath, '.sql'); - if (name.endsWith(".up")) { + if (name.endsWith('.up')) { return { id: name.slice(0, -3), - direction: "up", + direction: 'up', filePath, }; } - if (name.endsWith(".down")) { + if (name.endsWith('.down')) { return { id: name.slice(0, -5), - direction: "down", + direction: 'down', filePath, }; } return { id: name, - direction: "none", + direction: 'none', filePath, }; } @@ -308,9 +324,9 @@ function parseSqlFile(filePath: string): ParsedSqlFile { * Groups the SQL files by their significant part of the filename. * @param filePaths - The file paths to group. * @returns An array of SQL groups. - * + * * Throws an error if the files are not properly paired. - * + * */ function groupSqlFiles(filePaths: string[]): SqlGroup[] { const groups = new Map(); @@ -318,15 +334,18 @@ function groupSqlFiles(filePaths: string[]): SqlGroup[] { const parsed = parseSqlFile(filePath); if (!groups.has(parsed.id)) { - groups.set(parsed.id, {id: parsed.id}); + groups.set(parsed.id, { id: parsed.id }); } - const group = groups.get(parsed.id)!; + const group = groups.get(parsed.id); + if (!group) { + throw new Error(`No group found for ${parsed.id}`); + } - if (parsed.direction === "up") { + if (parsed.direction === 'up') { if (group.up) throw new Error(`Duplicate .up.sql for ${parsed.id}`); group.up = parsed.filePath; - } else if (parsed.direction === "down") { + } else if (parsed.direction === 'down') { if (group.down) throw new Error(`Duplicate .down.sql for ${parsed.id}`); group.down = parsed.filePath; } else { @@ -334,29 +353,29 @@ function groupSqlFiles(filePaths: string[]): SqlGroup[] { group.single = parsed.filePath; } } + for (const [id, group] of groups) { if (group.single && (group.up || group.down)) { throw new Error( `Conflicting SQL migration files for ${id}: cannot mix .sql with .up/.down` ); } - + if (group.down && !group.up) { - throw new Error( - `Found .down.sql without matching .up.sql for ${id}` - ); + throw new Error(`Found .down.sql without matching .up.sql for ${id}`); } } - return Array.from(groups.values()); + + return [...groups.values()]; } function sqlGroupId(group: SqlGroup): string { - let filePath = group.single ?? group.up ?? group.down; + const filePath = group.single ?? group.up ?? group.down; if (!filePath) { throw new Error(`No SQL file found for group ${group.id}`); } - return filePath.replace(/\.up\.sql$/, ".sql") - .replace(/\.down\.sql$/, ".sql"); + + return filePath.replace(/\.up\.sql$/, '.sql').replace(/\.down\.sql$/, '.sql'); } /** @@ -365,19 +384,19 @@ function sqlGroupId(group: SqlGroup): string { * @returns The migration unit. */ async function readSqlFileGroup(group: SqlGroup): Promise { - let actions: MigrationBuilderActions + let actions: MigrationBuilderActions; if (group.single) { actions = await sqlMigration(group.single); } else { - if (!group.up) { // Since a down migration without an up migration is a deviation from expected behaviour, we throw an error. throw new Error(`Missing .up.sql for ${group.id}`); } - const upSql = await readFile(group.up, "utf-8"); - const downSql = group.down ? await readFile(group.down, "utf-8") : undefined; + const upSql = await readFile(group.up, 'utf8'); + + const downSql = group.down ? await readFile(group.down, 'utf8') : undefined; actions = { up: (pgm) => pgm.sql(upSql), down: downSql ? (pgm) => pgm.sql(downSql) : undefined, @@ -385,15 +404,13 @@ async function readSqlFileGroup(group: SqlGroup): Promise { }; } - const filePaths = [ - group.up, - group.down, - group.single, - ].filter((p): p is string => Boolean(p)); + const filePaths = [group.up, group.down, group.single].filter( + (p): p is string => Boolean(p) + ); return { id: sqlGroupId(group), filePaths: filePaths, actions: actions, }; -} \ No newline at end of file +} diff --git a/src/runner.ts b/src/runner.ts index 84fb3f2b3..d45c776b9 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -4,9 +4,10 @@ import { db as Db } from './db'; import type { LogFn, Logger } from './logger'; import type { RunMigration } from './migration'; import { getMigrationFilePaths, Migration } from './migration'; +import type { MigrationLoaderConfig } from './migrationLoader'; +import { loadMigrationUnits } from './migrationLoader'; import type { ColumnDefinitions } from './operations/tables'; import { createSchemalize, getMigrationTableSchema, getSchemas } from './utils'; -import { loadMigrationUnits, type MigrationLoaderConfig } from './migrationLoader'; export interface RunnerOptionConfig { /** @@ -152,7 +153,8 @@ export interface RunnerOptionClient { dbClient: ClientBase; } -export type RunnerOption = RunnerOptionConfig & MigrationLoaderConfig & +export type RunnerOption = RunnerOptionConfig & + MigrationLoaderConfig & (RunnerOptionClient | RunnerOptionUrl); /** @@ -177,7 +179,7 @@ export async function loadMigrations( logger, }); const migrations: Migration[] = []; - + // Actual loading of files has been delegated to the loadMigrationUnits function. const migrationUnits = await loadMigrationUnits(options, absoluteFilePaths); for (const { id: filePath, actions } of migrationUnits) { diff --git a/test/jiti/jiti.spec.ts b/test/jiti/jiti.spec.ts index 1c03c8bc9..124a0e4cc 100644 --- a/test/jiti/jiti.spec.ts +++ b/test/jiti/jiti.spec.ts @@ -3,8 +3,8 @@ import { join, resolve } from 'node:path'; import { describe, expect, it, vi } from 'vitest'; import type { Migration, RunnerOption } from '../../src'; import type { DBConnection } from '../../src/db'; -import { loadMigrations } from '../../src/runner'; import { jiti } from '../../src/migrationLoader'; +import { loadMigrations } from '../../src/runner'; describe('loadMigrations', () => { it('should load migration files using jiti', async () => { diff --git a/test/migrationLoader.spec.ts b/test/migrationLoader.spec.ts index d1547bba8..a3ffa992d 100644 --- a/test/migrationLoader.spec.ts +++ b/test/migrationLoader.spec.ts @@ -1,16 +1,16 @@ -import { mkdtemp, rm, writeFile } from "node:fs/promises"; -import { join } from "node:path"; -import { tmpdir } from "node:os"; -import { describe, expect, it } from "vitest"; -import { - loadMigrationUnits, - type MigrationLoader, - type MigrationLoaderConfig, - type MigrationUnit, -} from "../src/migrationLoader"; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import type { + MigrationLoader, + MigrationLoaderConfig, + MigrationUnit, +} from '../src/migrationLoader'; +import { loadMigrationUnits } from '../src/migrationLoader'; async function withTempDir(run: (dir: string) => Promise): Promise { - const dir = await mkdtemp(join(tmpdir(), "npm-migration-loader-test-")); + const dir = await mkdtemp(join(tmpdir(), 'npm-migration-loader-test-')); try { return await run(dir); } finally { @@ -24,107 +24,114 @@ async function writeMigrationFile( content: string ): Promise { const path = join(dir, fileName); - await writeFile(path, content, "utf8"); + await writeFile(path, content, 'utf8'); return path; } -describe("loadMigrationUnits", () => { - it("keeps legacy SQL behavior by default (.up/.down are separate migrations)", async () => { +describe('loadMigrationUnits', () => { + it('keeps legacy SQL behavior by default (.up/.down are separate migrations)', async () => { await withTempDir(async (dir) => { const upPath = await writeMigrationFile( dir, - "001_create_users.up.sql", - "-- up migration\nCREATE TABLE users(id serial primary key);\n" + '001_create_users.up.sql', + '-- up migration\nCREATE TABLE users(id serial primary key);\n' ); const downPath = await writeMigrationFile( dir, - "001_create_users.down.sql", - "-- down migration\nDROP TABLE users;\n" + '001_create_users.down.sql', + '-- down migration\nDROP TABLE users;\n' ); const units = await loadMigrationUnits({}, [upPath, downPath]); expect(units).toHaveLength(2); - expect(units.map((u) => u.id)).toEqual([downPath, upPath].sort()); + expect(units.map((u) => u.id)).toEqual([downPath, upPath].toSorted()); expect(units.every((u) => u.filePaths.length === 1)).toBe(true); }); }); - it("groups .up/.down SQL files when the new sql loader is configured", async () => { + it('groups .up/.down SQL files when the new sql loader is configured', async () => { await withTempDir(async (dir) => { const upPath = await writeMigrationFile( dir, - "001_create_users.up.sql", - "CREATE TABLE users(id serial primary key);\n" + '001_create_users.up.sql', + 'CREATE TABLE users(id serial primary key);\n' ); const downPath = await writeMigrationFile( dir, - "001_create_users.down.sql", - "DROP TABLE users;\n" + '001_create_users.down.sql', + 'DROP TABLE users;\n' ); const config: MigrationLoaderConfig = { - migrationLoaderStrategies: [{ extensions: [".sql"], loader: "sql" }], + migrationLoaderStrategies: [{ extensions: ['.sql'], loader: 'sql' }], }; const units = await loadMigrationUnits(config, [upPath, downPath]); expect(units).toHaveLength(1); - expect(units[0].id).toBe(join(dir, "001_create_users.sql")); + expect(units[0].id).toBe(join(dir, '001_create_users.sql')); expect(units[0].filePaths).toEqual([upPath, downPath]); - expect(units[0].actions.up).toBeTypeOf("function"); - expect(units[0].actions.down).toBeTypeOf("function"); + expect(units[0].actions.up).toBeTypeOf('function'); + expect(units[0].actions.down).toBeTypeOf('function'); }); }); - it("throws when sql loader sees .down.sql without matching .up.sql", async () => { + it('throws when sql loader sees .down.sql without matching .up.sql', async () => { await withTempDir(async (dir) => { const downPath = await writeMigrationFile( dir, - "001_create_users.down.sql", - "DROP TABLE users;\n" + '001_create_users.down.sql', + 'DROP TABLE users;\n' ); const config: MigrationLoaderConfig = { - migrationLoaderStrategies: [{ extensions: [".sql"], loader: "sql" }], + migrationLoaderStrategies: [{ extensions: ['.sql'], loader: 'sql' }], }; await expect(loadMigrationUnits(config, [downPath])).rejects.toThrow( - "Found .down.sql without matching .up.sql for 001_create_users" + 'Found .down.sql without matching .up.sql for 001_create_users' ); }); }); - it("uses custom loader only for matched extension buckets", async () => { + it('uses custom loader only for matched extension buckets', async () => { await withTempDir(async (dir) => { const jsPath = await writeMigrationFile( dir, - "001_script.js", - "export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n" + '001_script.js', + 'export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n' ); const mjsPath = await writeMigrationFile( dir, - "002_script.mjs", - "export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n" + '002_script.mjs', + 'export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n' ); const customUnits: MigrationUnit[] = [ { - id: "custom-js-id", + id: 'custom-js-id', filePaths: [jsPath], - actions: { up: () => undefined, down: () => undefined, shorthands: {} }, + actions: { + up: () => {}, + down: () => {}, + shorthands: {}, + }, }, ]; - const customLoader: MigrationLoader = async () => customUnits; + + const customLoader: MigrationLoader = () => Promise.resolve(customUnits); const config: MigrationLoaderConfig = { - migrationLoaderStrategies: [{ extensions: [".js"], loader: customLoader }], + migrationLoaderStrategies: [ + { extensions: ['.js'], loader: customLoader }, + ], }; const units = await loadMigrationUnits(config, [jsPath, mjsPath]); const ids = units.map((u) => u.id); - expect(ids).toContain("custom-js-id"); + expect(ids).toContain('custom-js-id'); expect(ids).toContain(mjsPath); expect(units).toHaveLength(2); }); diff --git a/test/runner.loadMigrations.regression.spec.ts b/test/runner.loadMigrations.regression.spec.ts index 8bd5e01ec..fa77ac3ee 100644 --- a/test/runner.loadMigrations.regression.spec.ts +++ b/test/runner.loadMigrations.regression.spec.ts @@ -1,13 +1,13 @@ -import { mkdtemp, rm, writeFile } from "node:fs/promises"; -import { join } from "node:path"; -import { tmpdir } from "node:os"; -import { describe, expect, it } from "vitest"; -import type { RunnerOption } from "../src"; -import type { DBConnection } from "../src/db"; -import { loadMigrations } from "../src/runner"; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import type { RunnerOption } from '../src'; +import type { DBConnection } from '../src/db'; +import { loadMigrations } from '../src/runner'; async function withTempDir(run: (dir: string) => Promise): Promise { - const dir = await mkdtemp(join(tmpdir(), "npm-load-migrations-regression-")); + const dir = await mkdtemp(join(tmpdir(), 'npm-load-migrations-regression-')); try { return await run(dir); } finally { @@ -21,32 +21,32 @@ async function writeMigrationFile( content: string ): Promise { const path = join(dir, fileName); - await writeFile(path, content, "utf8"); + await writeFile(path, content, 'utf8'); return path; } -describe("loadMigrations regression behavior", () => { - it("keeps legacy .sql handling and preserves migration paths by default", async () => { +describe('loadMigrations regression behavior', () => { + it('keeps legacy .sql handling and preserves migration paths by default', async () => { await withTempDir(async (dir) => { const singleSqlPath = await writeMigrationFile( dir, - "001_single.sql", - "-- up migration\nSELECT 1;\n-- down migration\nSELECT 0;\n" + '001_single.sql', + '-- up migration\nSELECT 1;\n-- down migration\nSELECT 0;\n' ); const downSqlPath = await writeMigrationFile( dir, - "002_pair.down.sql", - "-- down migration\nDROP TABLE users;\n" + '002_pair.down.sql', + '-- down migration\nDROP TABLE users;\n' ); const upSqlPath = await writeMigrationFile( dir, - "002_pair.up.sql", - "-- up migration\nCREATE TABLE users(id serial primary key);\n" + '002_pair.up.sql', + '-- up migration\nCREATE TABLE users(id serial primary key);\n' ); const jsPath = await writeMigrationFile( dir, - "003_script.js", - "export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n" + '003_script.js', + 'export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n' ); const db: DBConnection = {} as never; @@ -54,9 +54,9 @@ describe("loadMigrations regression behavior", () => { dir, ignorePattern: undefined, useGlob: false, - migrationsTable: "migrations", - direction: "up", - databaseUrl: "postgres://user:pass@localhost/db", + migrationsTable: 'migrations', + direction: 'up', + databaseUrl: 'postgres://user:pass@localhost/db', }; const migrations = await loadMigrations(db, options, console); @@ -74,30 +74,30 @@ describe("loadMigrations regression behavior", () => { // Names are distinct for .up/.down pair under legacy default SQL handling. expect(migrations.map((migration) => migration.name)).toEqual([ - "001_single", - "002_pair.down", - "002_pair.up", - "003_script", + '001_single', + '002_pair.down', + '002_pair.up', + '003_script', ]); }); }); - it("keeps init before next when 001_init.sql is split into 001_init.up/down.sql", async () => { + it('keeps init before next when 001_init.sql is split into 001_init.up/down.sql', async () => { await withTempDir(async (dir) => { const initUpPath = await writeMigrationFile( dir, - "001_init.up.sql", - "CREATE TABLE init_table(id serial primary key);\n" + '001_init.up.sql', + 'CREATE TABLE init_table(id serial primary key);\n' ); const initDownPath = await writeMigrationFile( dir, - "001_init.down.sql", - "DROP TABLE init_table;\n" + '001_init.down.sql', + 'DROP TABLE init_table;\n' ); const nextPath = await writeMigrationFile( dir, - "002_next.sql", - "-- up migration\nSELECT 2;\n" + '002_next.sql', + '-- up migration\nSELECT 2;\n' ); const db: DBConnection = {} as never; @@ -105,41 +105,41 @@ describe("loadMigrations regression behavior", () => { dir, ignorePattern: undefined, useGlob: false, - migrationsTable: "migrations", - direction: "up", - databaseUrl: "postgres://user:pass@localhost/db", - migrationLoaderStrategies: [{ extensions: [".sql"], loader: "sql" }], + migrationsTable: 'migrations', + direction: 'up', + databaseUrl: 'postgres://user:pass@localhost/db', + migrationLoaderStrategies: [{ extensions: ['.sql'], loader: 'sql' }], }; const migrations = await loadMigrations(db, options, console); expect(migrations).toHaveLength(2); expect(migrations.map((migration) => migration.path)).toEqual([ - join(dir, "001_init.sql"), + join(dir, '001_init.sql'), nextPath, ]); expect(migrations.map((migration) => migration.name)).toEqual([ - "001_init", - "002_next", + '001_init', + '002_next', ]); // Ensure files are truly split and still represented as one ordered migration. - expect(initUpPath).toContain(".up.sql"); - expect(initDownPath).toContain(".down.sql"); + expect(initUpPath).toContain('.up.sql'); + expect(initDownPath).toContain('.down.sql'); }); }); - it("falls back to default loader for unmatched extensions when only .sql strategy is configured", async () => { + it('falls back to default loader for unmatched extensions when only .sql strategy is configured', async () => { await withTempDir(async (dir) => { const sqlPath = await writeMigrationFile( dir, - "001_init.sql", - "-- up migration\nSELECT 1;\n" + '001_init.sql', + '-- up migration\nSELECT 1;\n' ); const jsPath = await writeMigrationFile( dir, - "002_next.js", - "export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n" + '002_next.js', + 'export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n' ); const db: DBConnection = {} as never; @@ -147,10 +147,10 @@ describe("loadMigrations regression behavior", () => { dir, ignorePattern: undefined, useGlob: false, - migrationsTable: "migrations", - direction: "up", - databaseUrl: "postgres://user:pass@localhost/db", - migrationLoaderStrategies: [{ extensions: [".sql"], loader: "sql" }], + migrationsTable: 'migrations', + direction: 'up', + databaseUrl: 'postgres://user:pass@localhost/db', + migrationLoaderStrategies: [{ extensions: ['.sql'], loader: 'sql' }], }; const migrations = await loadMigrations(db, options, console); @@ -159,23 +159,23 @@ describe("loadMigrations regression behavior", () => { jsPath, ]); expect(migrations.map((migration) => migration.name)).toEqual([ - "001_init", - "002_next", + '001_init', + '002_next', ]); }); }); - it("throws when .sql and .up/.down forms are mixed for the same migration id", async () => { + it('throws when .sql and .up/.down forms are mixed for the same migration id', async () => { await withTempDir(async (dir) => { await writeMigrationFile( dir, - "001_init.sql", - "-- up migration\nSELECT 1;\n" + '001_init.sql', + '-- up migration\nSELECT 1;\n' ); await writeMigrationFile( dir, - "001_init.up.sql", - "CREATE TABLE t(id serial primary key);\n" + '001_init.up.sql', + 'CREATE TABLE t(id serial primary key);\n' ); const db: DBConnection = {} as never; @@ -183,24 +183,24 @@ describe("loadMigrations regression behavior", () => { dir, ignorePattern: undefined, useGlob: false, - migrationsTable: "migrations", - direction: "up", - databaseUrl: "postgres://user:pass@localhost/db", - migrationLoaderStrategies: [{ extensions: [".sql"], loader: "sql" }], + migrationsTable: 'migrations', + direction: 'up', + databaseUrl: 'postgres://user:pass@localhost/db', + migrationLoaderStrategies: [{ extensions: ['.sql'], loader: 'sql' }], }; await expect(loadMigrations(db, options, console)).rejects.toThrow( - "Conflicting SQL migration files for 001_init: cannot mix .sql with .up/.down" + 'Conflicting SQL migration files for 001_init: cannot mix .sql with .up/.down' ); }); }); - it("matches strategy extensions case-insensitively", async () => { + it('matches strategy extensions case-insensitively', async () => { await withTempDir(async (dir) => { const jsPath = await writeMigrationFile( dir, - "001_case.js", - "export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n" + '001_case.js', + 'export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n' ); const db: DBConnection = {} as never; @@ -208,16 +208,16 @@ describe("loadMigrations regression behavior", () => { dir, ignorePattern: undefined, useGlob: false, - migrationsTable: "migrations", - direction: "up", - databaseUrl: "postgres://user:pass@localhost/db", - migrationLoaderStrategies: [{ extensions: [".JS"], loader: "default" }], + migrationsTable: 'migrations', + direction: 'up', + databaseUrl: 'postgres://user:pass@localhost/db', + migrationLoaderStrategies: [{ extensions: ['.JS'], loader: 'default' }], }; const migrations = await loadMigrations(db, options, console); expect(migrations).toHaveLength(1); expect(migrations[0].path).toBe(jsPath); - expect(migrations[0].name).toBe("001_case"); + expect(migrations[0].name).toBe('001_case'); }); }); }); From 467b9dd9b22d97e7916f830c7bf4184e7702ec1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9tur=20=C3=9E=C3=B3r=20Valdimarsson?= Date: Thu, 19 Mar 2026 20:25:29 +0100 Subject: [PATCH 3/8] Use correct sorting strategy for filenames. --- src/migration.ts | 15 ++------ src/migrationLoader.ts | 3 +- src/utils/index.ts | 1 + src/utils/stringComparison.ts | 8 ++++ test/migrationLoader.spec.ts | 58 ++++++++++++++++++++++++++++- test/utils/stringComparison.spec.ts | 12 ++++++ 6 files changed, 84 insertions(+), 13 deletions(-) create mode 100644 src/utils/stringComparison.ts create mode 100644 test/utils/stringComparison.spec.ts diff --git a/src/migration.ts b/src/migration.ts index 8859482c7..23cf2f5e4 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -10,8 +10,10 @@ import { MigrationBuilder } from './migrationBuilder'; import type { ColumnDefinitions } from './operations/tables'; import type { MigrationDirection, RunnerOption } from './runner'; import type { MigrationBuilderActions } from './sqlMigration'; -import { getMigrationTableSchema } from './utils'; - +import { + getMigrationTableSchema, + localeCompareStringsNumerically, +} from './utils'; /* * A new Migration is instantiated for each migration file. * @@ -55,15 +57,6 @@ export type CreateOptions = { const SEPARATOR = '_'; -function localeCompareStringsNumerically(a: string, b: string): number { - return a.localeCompare(b, undefined, { - usage: 'sort', - numeric: true, - sensitivity: 'variant', - ignorePunctuation: true, - }); -} - function compareFileNamesByTimestamp( a: string, b: string, diff --git a/src/migrationLoader.ts b/src/migrationLoader.ts index 5d491706a..f019ff05a 100644 --- a/src/migrationLoader.ts +++ b/src/migrationLoader.ts @@ -3,6 +3,7 @@ import { readFile } from 'node:fs/promises'; import { basename, extname } from 'node:path'; import type { MigrationBuilderActions } from './sqlMigration'; import { sqlMigration } from './sqlMigration'; +import { localeCompareStringsNumerically } from './utils/stringComparison'; /*** * Migration loader module. @@ -259,7 +260,7 @@ export async function loadMigrationUnits( // Since the sql migration loader modifies the id, it is no longer comparable. Hence we sort by the file path of the first file in the unit that always exists. const sortedMigrationUnits = migrationUnits.toSorted((a, b) => - a.filePaths[0].localeCompare(b.filePaths[0]) + localeCompareStringsNumerically(a.filePaths[0], b.filePaths[0]) ); return sortedMigrationUnits; } diff --git a/src/utils/index.ts b/src/utils/index.ts index 05d915861..006e052a0 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -13,6 +13,7 @@ export { makeComment } from './makeComment'; export { PgLiteral, isPgLiteral } from './PgLiteral'; export type { PgLiteralValue } from './PgLiteral'; export { quote } from './quote'; +export { localeCompareStringsNumerically } from './stringComparison'; export { stringIdGenerator } from './stringIdGenerator'; export { toArray } from './toArray'; export { applyType, applyTypeAdapters } from './types'; diff --git a/src/utils/stringComparison.ts b/src/utils/stringComparison.ts new file mode 100644 index 000000000..3a1ee4ee4 --- /dev/null +++ b/src/utils/stringComparison.ts @@ -0,0 +1,8 @@ +export function localeCompareStringsNumerically(a: string, b: string): number { + return a.localeCompare(b, undefined, { + usage: 'sort', + numeric: true, + sensitivity: 'variant', + ignorePunctuation: true, + }); +} diff --git a/test/migrationLoader.spec.ts b/test/migrationLoader.spec.ts index a3ffa992d..ec85003e8 100644 --- a/test/migrationLoader.spec.ts +++ b/test/migrationLoader.spec.ts @@ -1,6 +1,7 @@ +import { readdirSync } from 'node:fs'; import { mkdtemp, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { basename, join, resolve } from 'node:path'; import { describe, expect, it } from 'vitest'; import type { MigrationLoader, @@ -136,4 +137,59 @@ describe('loadMigrationUnits', () => { expect(units).toHaveLength(2); }); }); + + it('sorts cockroach files up to 062 in numeric order and keeps 062_view before 062_view_test', async () => { + const customLoader: MigrationLoader = (filePaths) => + Promise.resolve( + filePaths.map((filePath) => ({ + id: filePath, + filePaths: [filePath], + actions: { + up: () => {}, + down: () => {}, + shorthands: {}, + }, + })) + ); + + const config: MigrationLoaderConfig = { + migrationLoaderStrategies: [ + { extensions: ['.js'], loader: customLoader }, + ], + }; + + const cockroachDir = resolve(import.meta.dirname, 'cockroach'); + const filePaths = readdirSync(cockroachDir) + .filter((fileName) => { + const match = /^(\d+)_.*\.js$/.exec(fileName); + if (!match) { + return false; + } + + const prefix = Number(match[1]); + return prefix <= 62; + }) + .toSorted() + .map((fileName) => join(cockroachDir, fileName)); + + const units = await loadMigrationUnits(config, filePaths); + const sortedNames = units.map((unit) => basename(unit.filePaths[0])); + const sortedPrefixes = sortedNames.map((fileName) => + Number((/^(\d+)_/.exec(fileName) ?? [])[1]) + ); + + expect(sortedPrefixes).toEqual(sortedPrefixes.toSorted((a, b) => a - b)); + + expect(sortedNames.indexOf('062_view.js')).toBeGreaterThanOrEqual(0); + expect(sortedNames.indexOf('062_view_test.js')).toBeGreaterThanOrEqual(0); + expect(sortedNames.indexOf('062_view.js')).toBeLessThan( + sortedNames.indexOf('062_view_test.js') + ); + + // Regression check for the observed bug. + expect(units.slice(-2).map((unit) => unit.filePaths[0])).toEqual([ + join(cockroachDir, '062_view.js'), + join(cockroachDir, '062_view_test.js'), + ]); + }); }); diff --git a/test/utils/stringComparison.spec.ts b/test/utils/stringComparison.spec.ts new file mode 100644 index 000000000..4babc28c4 --- /dev/null +++ b/test/utils/stringComparison.spec.ts @@ -0,0 +1,12 @@ +import { describe, expect, it } from 'vitest'; +import { localeCompareStringsNumerically } from '../../src/utils'; + +describe('localeCompareStringsNumerically', () => { + it('sorts 062_view before 062_view_test', () => { + const files = ['062_view_test.js', '062_view.js']; + + const sorted = files.toSorted(localeCompareStringsNumerically); + + expect(sorted).toEqual(['062_view.js', '062_view_test.js']); + }); +}); From 6f93c26bc87e48c9b7aee753b1d669b01886ada2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9tur=20=C3=9E=C3=B3r=20Valdimarsson?= Date: Fri, 20 Mar 2026 09:29:10 +0100 Subject: [PATCH 4/8] Use a single jiti instance. --- bin/node-pg-migrate.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bin/node-pg-migrate.ts b/bin/node-pg-migrate.ts index cd3e7aace..634dd962f 100755 --- a/bin/node-pg-migrate.ts +++ b/bin/node-pg-migrate.ts @@ -4,13 +4,13 @@ import type { DotenvConfigOptions } from 'dotenv'; // Import as node-pg-migrate, so tsup does not self-reference as '../dist' // otherwise this could not be imported by esm // @ts-ignore: when a clean was made, the types are not present in the first run -import { createJiti } from 'jiti'; import type { RunnerOption } from 'node-pg-migrate'; import { Migration, PG_MIGRATE_LOCK_ID, runner as migrationRunner, } from 'node-pg-migrate'; +import { jiti } from 'node-pg-migrate/migrationLoader'; import { join, resolve } from 'node:path'; import { cwd } from 'node:process'; import { format } from 'node:util'; @@ -404,7 +404,6 @@ process.env.SUPPRESS_NO_CONFIG_WARNING = oldSuppressWarning; const configFileName: string | undefined = argv[configFileArg]; if (configFileName) { - const jiti = createJiti(process.cwd()); const configModule: unknown = await jiti.import(resolve(configFileName)); let json: unknown; From 681054b8ebf7474766df0c1c0a5555f8697da511 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9tur=20=C3=9E=C3=B3r=20Valdimarsson?= Date: Fri, 20 Mar 2026 09:32:12 +0100 Subject: [PATCH 5/8] Ensure migration file sorting is uniform. Corrections of how jiti is exported to node-pg-migrate for legacy build. --- bin/node-pg-migrate.ts | 4 +- src/index.ts | 1 + src/migration.ts | 71 ++------------------ src/migrationLoader.ts | 11 ++- src/utils/comparators.ts | 52 ++++++++++++++ src/utils/createTransformer.ts | 2 +- src/utils/fileNameUtils.ts | 54 +++++++++++++++ src/utils/index.ts | 7 +- src/utils/stringComparison.ts | 8 --- test/migration.spec.ts | 2 +- test/migrationLoader.spec.ts | 5 +- test/utils/filenameComparators.spec.ts | 93 ++++++++++++++++++++++++++ 12 files changed, 226 insertions(+), 84 deletions(-) create mode 100644 src/utils/comparators.ts create mode 100644 src/utils/fileNameUtils.ts delete mode 100644 src/utils/stringComparison.ts create mode 100644 test/utils/filenameComparators.spec.ts diff --git a/bin/node-pg-migrate.ts b/bin/node-pg-migrate.ts index 634dd962f..5d98332c4 100755 --- a/bin/node-pg-migrate.ts +++ b/bin/node-pg-migrate.ts @@ -6,11 +6,11 @@ import type { DotenvConfigOptions } from 'dotenv'; // @ts-ignore: when a clean was made, the types are not present in the first run import type { RunnerOption } from 'node-pg-migrate'; import { + jiti, Migration, - PG_MIGRATE_LOCK_ID, runner as migrationRunner, + PG_MIGRATE_LOCK_ID, } from 'node-pg-migrate'; -import { jiti } from 'node-pg-migrate/migrationLoader'; import { join, resolve } from 'node:path'; import { cwd } from 'node:process'; import { format } from 'node:util'; diff --git a/src/index.ts b/src/index.ts index 2425144ed..953d8edf6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,5 +1,6 @@ export { Migration } from './migration'; export { MigrationBuilder } from './migrationBuilder'; +export { jiti } from './migrationLoader'; export type { CreateCast, CreateCastFn, diff --git a/src/migration.ts b/src/migration.ts index 23cf2f5e4..bdec0ff0a 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -11,8 +11,10 @@ import type { ColumnDefinitions } from './operations/tables'; import type { MigrationDirection, RunnerOption } from './runner'; import type { MigrationBuilderActions } from './sqlMigration'; import { + compareMigrationFileNames, getMigrationTableSchema, - localeCompareStringsNumerically, + getNumericPrefix, + getSuffixFromFileName, } from './utils'; /* * A new Migration is instantiated for each migration file. @@ -57,17 +59,6 @@ export type CreateOptions = { const SEPARATOR = '_'; -function compareFileNamesByTimestamp( - a: string, - b: string, - logger?: Logger -): number { - const aTimestamp = getNumericPrefix(a, logger); - const bTimestamp = getNumericPrefix(b, logger); - - return aTimestamp - bTimestamp; -} - interface LoadMigrationFilesOptions { /** * Regex pattern for file names to ignore (ignores files starting with `.` by default). @@ -130,11 +121,7 @@ export async function getMigrationFilePaths( }); return globMatches - .toSorted( - (a, b) => - compareFileNamesByTimestamp(a.name, b.name, logger) || - localeCompareStringsNumerically(a.name, b.name) - ) + .toSorted((a, b) => compareMigrationFileNames(a.name, b.name, logger)) .map((pathScurry) => pathScurry.fullpath()); } @@ -155,18 +142,10 @@ export async function getMigrationFilePaths( (dirent.isFile() || dirent.isSymbolicLink()) && !ignoreRegexp.test(dirent.name) ) - .toSorted( - (a, b) => - compareFileNamesByTimestamp(a.name, b.name, logger) || - localeCompareStringsNumerically(a.name, b.name) - ) + .toSorted((a, b) => compareMigrationFileNames(a.name, b.name, logger)) .map((dirent) => resolve(dir, dirent.name)); } -function getSuffixFromFileName(fileName: string): string { - return extname(fileName).slice(1); -} - async function getLastSuffix( dir: string, ignorePattern?: string @@ -181,46 +160,6 @@ async function getLastSuffix( } } -/** - * Extracts numeric value from everything in `filename` before `SEPARATOR`. - * 17 digit numbers are interpreted as UTC date and converted to the number - * representation of that date. 1...4 digit numbers are interpreted as index - * based naming scheme. - * - * @param filename filename to extract the prefix from - * @param logger Redirect messages to this logger object, rather than `console`. - * @returns numeric value of the filename prefix (everything before `SEPARATOR`). - */ -export function getNumericPrefix( - filename: string, - logger: Logger = console -): number { - const prefix = (/^(\d+)/.exec(filename) || '')[0]; - const value = Number(prefix); - - if (!/^\d+$/.test(prefix) || Number.isNaN(value)) { - logger.error(`Cannot determine numeric prefix for "${filename}"`); - throw new Error(`Cannot determine numeric prefix for "${filename}"`); - } - - // Special case for UTC timestamp - if (prefix.length === 17) { - // utc: 20200513070724505 - const year = prefix.slice(0, 4); - const month = prefix.slice(4, 6); - const date = prefix.slice(6, 8); - const hours = prefix.slice(8, 10); - const minutes = prefix.slice(10, 12); - const seconds = prefix.slice(12, 14); - const ms = prefix.slice(14, 17); - return new Date( - `${year}-${month}-${date}T${hours}:${minutes}:${seconds}.${ms}Z` - ).valueOf(); - } - - return value; -} - async function resolveSuffix( directory: string, options: CreateOptionsDefault diff --git a/src/migrationLoader.ts b/src/migrationLoader.ts index f019ff05a..d0ff2311a 100644 --- a/src/migrationLoader.ts +++ b/src/migrationLoader.ts @@ -1,9 +1,10 @@ import { createJiti } from 'jiti'; import { readFile } from 'node:fs/promises'; import { basename, extname } from 'node:path'; +import type { Logger } from './logger'; import type { MigrationBuilderActions } from './sqlMigration'; import { sqlMigration } from './sqlMigration'; -import { localeCompareStringsNumerically } from './utils/stringComparison'; +import { compareMigrationFileNames } from './utils'; /*** * Migration loader module. @@ -72,6 +73,11 @@ export interface MigrationLoaderConfig { * If no strategy matches, the default strategy is used. */ migrationLoaderStrategies?: MigrationLoaderStrategy[]; + + /** + * Redirect messages to this logger object, rather than `console`. + */ + logger?: Logger; } /** @@ -258,9 +264,8 @@ export async function loadMigrationUnits( migrationUnits.push(...units); } - // Since the sql migration loader modifies the id, it is no longer comparable. Hence we sort by the file path of the first file in the unit that always exists. const sortedMigrationUnits = migrationUnits.toSorted((a, b) => - localeCompareStringsNumerically(a.filePaths[0], b.filePaths[0]) + compareMigrationFileNames(basename(a.id), basename(b.id), config.logger) ); return sortedMigrationUnits; } diff --git a/src/utils/comparators.ts b/src/utils/comparators.ts new file mode 100644 index 000000000..a1becd67d --- /dev/null +++ b/src/utils/comparators.ts @@ -0,0 +1,52 @@ +import type { Logger } from '../logger'; +import { getNumericPrefix } from './fileNameUtils'; + +export function localeCompareStringsNumerically(a: string, b: string): number { + return a.localeCompare(b, undefined, { + usage: 'sort', + numeric: true, + sensitivity: 'variant', + ignorePunctuation: true, + }); +} + +/** + * Compares two file names by their numeric prefix. + * 17 digit numbers are interpreted as UTC date and converted to the number + * representation of that date. 1...4 digit numbers are interpreted as index + * based naming scheme. + * + * @param a the first file name to compare + * @param b the second file name to compare + * @param logger the logger to use for logging + * @returns a negative value if `a` is less than `b`, 0 if they are equal, and a positive value if `a` is greater than `b` + */ +export function compareFileNamesByTimestamp( + a: string, + b: string, + logger?: Logger +): number { + const aTimestamp = getNumericPrefix(a, logger); + const bTimestamp = getNumericPrefix(b, logger); + + return aTimestamp - bTimestamp; +} + +/** + * Compares two file names by their numeric prefix and locale. + * + * @param a the first file name to compare + * @param b the second file name to compare + * @param logger the logger to use for logging + * @returns a negative value if `a` is less than `b`, 0 if they are equal, and a positive value if `a` is greater than `b` + */ +export function compareMigrationFileNames( + a: string, + b: string, + logger?: Logger +): number { + return ( + compareFileNamesByTimestamp(a, b, logger) || + localeCompareStringsNumerically(a, b) + ); +} diff --git a/src/utils/createTransformer.ts b/src/utils/createTransformer.ts index dba1c1de4..0fd0f5cd5 100644 --- a/src/utils/createTransformer.ts +++ b/src/utils/createTransformer.ts @@ -1,6 +1,6 @@ -import { escapeValue } from '.'; import type { Name, Value } from '../operations/generalTypes'; import { isNameObject } from '../operations/generalTypes'; +import { escapeValue } from './escapeValue'; export type Literal = (v: Name) => string; diff --git a/src/utils/fileNameUtils.ts b/src/utils/fileNameUtils.ts new file mode 100644 index 000000000..0b0ab9455 --- /dev/null +++ b/src/utils/fileNameUtils.ts @@ -0,0 +1,54 @@ +import { extname } from 'node:path'; +import type { Logger } from '../logger'; + +/** + * Extracts the file extension from a file name. + * + * @param fileName file name to extract the extension from + * @returns the file extension + */ +export function getSuffixFromFileName(fileName: string): string { + return extname(fileName).slice(1); +} + +/** + * Extracts contiguous numeric value from the start of `filename` up to the + * first non-numeric character. + * 17 digit numbers are interpreted as UTC date and converted to the number + * representation of that date. 1...4 digit numbers are interpreted as index + * based naming scheme. + * + * @param filename filename to extract the prefix from + * @param logger Redirect messages to this logger object, rather than `console`. + * @returns numeric value of the filename prefix (everything before the first + * non-numeric character). + */ +export function getNumericPrefix( + filename: string, + logger: Logger = console +): number { + const prefix = (/^(\d+)/.exec(filename) || '')[0]; + const value = Number(prefix); + + if (!/^\d+$/.test(prefix) || Number.isNaN(value)) { + logger.error(`Cannot determine numeric prefix for "${filename}"`); + throw new Error(`Cannot determine numeric prefix for "${filename}"`); + } + + // Special case for UTC timestamp + if (prefix.length === 17) { + // utc: 20200513070724505 + const year = prefix.slice(0, 4); + const month = prefix.slice(4, 6); + const date = prefix.slice(6, 8); + const hours = prefix.slice(8, 10); + const minutes = prefix.slice(10, 12); + const seconds = prefix.slice(12, 14); + const ms = prefix.slice(14, 17); + return new Date( + `${year}-${month}-${date}T${hours}:${minutes}:${seconds}.${ms}Z` + ).valueOf(); + } + + return value; +} diff --git a/src/utils/index.ts b/src/utils/index.ts index 006e052a0..6559785d5 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -1,7 +1,13 @@ +export { + compareFileNamesByTimestamp, + compareMigrationFileNames, + localeCompareStringsNumerically, +} from './comparators'; export { createSchemalize } from './createSchemalize'; export { createTransformer } from './createTransformer'; export { decamelize } from './decamelize'; export { escapeValue } from './escapeValue'; +export { getNumericPrefix, getSuffixFromFileName } from './fileNameUtils'; export { formatLines } from './formatLines'; export { formatParams } from './formatParams'; export { formatPartitionColumns } from './formatPartitionColumns'; @@ -13,7 +19,6 @@ export { makeComment } from './makeComment'; export { PgLiteral, isPgLiteral } from './PgLiteral'; export type { PgLiteralValue } from './PgLiteral'; export { quote } from './quote'; -export { localeCompareStringsNumerically } from './stringComparison'; export { stringIdGenerator } from './stringIdGenerator'; export { toArray } from './toArray'; export { applyType, applyTypeAdapters } from './types'; diff --git a/src/utils/stringComparison.ts b/src/utils/stringComparison.ts deleted file mode 100644 index 3a1ee4ee4..000000000 --- a/src/utils/stringComparison.ts +++ /dev/null @@ -1,8 +0,0 @@ -export function localeCompareStringsNumerically(a: string, b: string): number { - return a.localeCompare(b, undefined, { - usage: 'sort', - numeric: true, - sensitivity: 'variant', - ignorePunctuation: true, - }); -} diff --git a/test/migration.spec.ts b/test/migration.spec.ts index db2b8a500..c6cf57472 100644 --- a/test/migration.spec.ts +++ b/test/migration.spec.ts @@ -7,9 +7,9 @@ import type { Logger } from '../src/logger'; import { FilenameFormat, getMigrationFilePaths, - getNumericPrefix, Migration, } from '../src/migration'; +import { getNumericPrefix } from '../src/utils'; const callbackMigration = '1414549381268_names.js'; const promiseMigration = '1414549381268_names_promise.js'; diff --git a/test/migrationLoader.spec.ts b/test/migrationLoader.spec.ts index ec85003e8..4d075a821 100644 --- a/test/migrationLoader.spec.ts +++ b/test/migrationLoader.spec.ts @@ -109,9 +109,10 @@ describe('loadMigrationUnits', () => { 'export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n' ); + const customId = '000_custom-js-id'; const customUnits: MigrationUnit[] = [ { - id: 'custom-js-id', + id: customId, filePaths: [jsPath], actions: { up: () => {}, @@ -132,7 +133,7 @@ describe('loadMigrationUnits', () => { const units = await loadMigrationUnits(config, [jsPath, mjsPath]); const ids = units.map((u) => u.id); - expect(ids).toContain('custom-js-id'); + expect(ids).toContain(customId); expect(ids).toContain(mjsPath); expect(units).toHaveLength(2); }); diff --git a/test/utils/filenameComparators.spec.ts b/test/utils/filenameComparators.spec.ts new file mode 100644 index 000000000..a59d8801b --- /dev/null +++ b/test/utils/filenameComparators.spec.ts @@ -0,0 +1,93 @@ +import { describe, expect, it, vi } from 'vitest'; +import type { Logger } from '../../src/logger'; +import { + compareFileNamesByTimestamp, + compareMigrationFileNames, + getSuffixFromFileName, +} from '../../src/utils'; + +describe('getSuffixFromFileName', () => { + it('extracts extension without the leading dot', () => { + expect(getSuffixFromFileName('001_migration.js')).toBe('js'); + expect(getSuffixFromFileName('001_migration.sql')).toBe('sql'); + }); + + it('returns empty string when there is no extension', () => { + expect(getSuffixFromFileName('no-extension')).toBe(''); + }); + + it('extracts only the last extension', () => { + expect(getSuffixFromFileName('archive.tar.gz')).toBe('gz'); + }); +}); + +describe('compareFileNamesByTimestamp', () => { + const logger: Logger = { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }; + + it('compares by numeric prefix (index / timestamp modes)', () => { + expect(compareFileNamesByTimestamp('2_a.js', '1_b.js')).toBeGreaterThan(0); + expect(compareFileNamesByTimestamp('1_b.js', '2_a.js')).toBeLessThan(0); + }); + + it('returns 0 when the numeric prefixes match', () => { + expect(compareFileNamesByTimestamp('0001_a.js', '1_b.js')).toBe(0); + }); + + it('throws when the first file has no numeric prefix', () => { + expect(() => + compareFileNamesByTimestamp('invalid-prefix.js', '1_b.js', logger) + ).toThrow( + new Error('Cannot determine numeric prefix for "invalid-prefix.js"') + ); + }); +}); + +describe('compareMigrationFileNames', () => { + const logger: Logger = { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }; + + it('orders by numeric prefix when they differ', () => { + expect( + compareMigrationFileNames('2_a.js', '1_b.js', logger) + ).toBeGreaterThan(0); + expect(compareMigrationFileNames('1_b.js', '2_a.js', logger)).toBeLessThan( + 0 + ); + }); + + it('falls back to locale numeric compare for equal numeric prefixes', () => { + const files = ['062_view_test.js', '062_view.js'].toSorted((a, b) => + compareMigrationFileNames(a, b) + ); + + expect(files).toEqual(['062_view.js', '062_view_test.js']); + }); + + it('orders by UTC timestamp when the numeric prefix is 17 digits', () => { + const t1 = '20200513070724505'; + const t2 = '20200513070724506'; + + const a = `${t1}_a.sql`; + const b = `${t2}_b.sql`; + + expect(compareMigrationFileNames(a, b, logger)).toBeLessThan(0); + expect(compareMigrationFileNames(b, a, logger)).toBeGreaterThan(0); + }); + + it('throws when the first file has no numeric prefix', () => { + expect(() => + compareMigrationFileNames('invalid-prefix.js', '1_b.js', logger) + ).toThrow( + new Error('Cannot determine numeric prefix for "invalid-prefix.js"') + ); + }); +}); From b2ae8bcb20d010673610fb4f0da64ae2757181f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9tur=20=C3=9E=C3=B3r=20Valdimarsson?= Date: Fri, 20 Mar 2026 10:37:46 +0100 Subject: [PATCH 6/8] Minor fixes: Avoid exhausting file handles. Documentation updates. Clarification of SQL migration loader behaviour. Added more tests for utilities and loader. Moved tests to separate files. Renamed testfiles to follow test target file name. "Nits" addressed. --- docs/src/api.md | 2 +- docs/src/migration-loading-strategies.md | 7 +- src/migrationLoader.ts | 71 ++++++++---- test/migration.spec.ts | 34 ------ test/migrationLoader.spec.ts | 105 ++++++++++++++++++ ...omparators.spec.ts => comparators.spec.ts} | 25 +++-- test/utils/fileNameUtils.spec.ts | 64 +++++++++++ test/utils/stringComparison.spec.ts | 12 -- 8 files changed, 237 insertions(+), 83 deletions(-) rename test/utils/{filenameComparators.spec.ts => comparators.spec.ts} (77%) create mode 100644 test/utils/fileNameUtils.spec.ts delete mode 100644 test/utils/stringComparison.spec.ts diff --git a/docs/src/api.md b/docs/src/api.md index 395d835f6..ebc20fb9a 100644 --- a/docs/src/api.md +++ b/docs/src/api.md @@ -33,7 +33,7 @@ which takes options argument with the following structure (similar to [command l | `log` | `function` | Redirect log messages to this function, rather than `console` | | `logger` | `object with debug/info/warn/error methods` | Redirect messages to this logger object, rather than `console` | | `verbose` | `boolean` | Print all debug messages like DB queries run (if you switch it on, it will disable `logger.debug` method) | -| `decamelize` | `boolean` | Runs `[decamelize](https://github.com/salsita/node-pg-migrate/blob/main/src/utils/decamelize.ts)` on table/column/etc. names | +| `decamelize` | `boolean` | Runs [`decamelize`](https://github.com/salsita/node-pg-migrate/blob/main/src/utils/decamelize.ts) on table/column/etc. names | | `migrationLoaderStrategies` | `MigrationLoaderStrategy[]` | Allows custom loading strategies based on file extensions. If omitted, default behavior is used. See [Migration Loading Strategies](migration-loading-strategies). | ### MigrationLoaderStrategy diff --git a/docs/src/migration-loading-strategies.md b/docs/src/migration-loading-strategies.md index 10c9df6f9..cf22a56f5 100644 --- a/docs/src/migration-loading-strategies.md +++ b/docs/src/migration-loading-strategies.md @@ -44,6 +44,7 @@ await runner({ With this configuration: - `001_init.up.sql` + `001_init.down.sql` are treated as one migration (`001_init`) +- The migration `id` is normalized to the equivalent `.sql` form (`001_init.up.sql` / `001_init.down.sql` -> `001_init.sql`). This means you can switch from a single `001_init.sql` migration to split `.up/.down` files (or vice versa) without creating a second entry in `migrationsTable`. - `001_init.sql` still works as a single-file SQL migration - mixing `001_init.sql` with `001_init.up.sql` / `001_init.down.sql` throws an error @@ -78,9 +79,9 @@ await runner({ - Each strategy handles one or more extensions - If no strategy matches an extension, the `default` loader is used -# Legacy SQL migrations +## Legacy SQL migrations -## Why it exists +### Why it exists The legacy SQL loader has been supported for a long time, even when it was less visible in the docs. @@ -91,7 +92,7 @@ Common use cases include: So if your team already relies on plain `.sql` files, that workflow is still supported. -## Markers and default fallback +### Markers and default fallback The classic SQL template uses marker comments: diff --git a/src/migrationLoader.ts b/src/migrationLoader.ts index d0ff2311a..bf04ec31d 100644 --- a/src/migrationLoader.ts +++ b/src/migrationLoader.ts @@ -91,7 +91,7 @@ export interface MigrationLoaderStrategy { /** * Loader that handles conversion of file paths to migration units. - * Can be a loader function, when used as an API, or a predefined loader when using a json configuration file. + * Can be a loader function, when used as an API, or a predefined loader when using a json configuration file. * * @param filePaths - The file paths to load migrations from. * @returns The migration units. @@ -109,16 +109,19 @@ export interface MigrationLoaderStrategy { */ export function createDefaultMigrationLoader(): MigrationLoader { const loader: MigrationLoader = async (filePaths: string[]) => { - const migrationUnits: MigrationUnit[] = await Promise.all( - filePaths.map(async (filePath) => { - const action: MigrationBuilderActions = await jiti.import(filePath); - return { - id: filePath, - filePaths: [filePath], - actions: action, - }; - }) - ); + const migrationUnits: MigrationUnit[] = []; + + // Load migrations sequentially to avoid unnecessary parallelism and + // potential handle pressure from many concurrent imports. + for (const filePath of filePaths) { + const action: MigrationBuilderActions = await jiti.import(filePath); + migrationUnits.push({ + id: filePath, + filePaths: [filePath], + actions: action, + }); + } + return migrationUnits; }; @@ -131,16 +134,19 @@ export function createDefaultMigrationLoader(): MigrationLoader { */ export function createLegacySqlMigrationLoader(): MigrationLoader { const loader: MigrationLoader = async (filePaths: string[]) => { - const migrationUnits: MigrationUnit[] = await Promise.all( - filePaths.map(async (filePath) => { - const actions = await sqlMigration(filePath); - return { - id: filePath, - filePaths: [filePath], - actions: actions, - }; - }) - ); + const migrationUnits: MigrationUnit[] = []; + + // Load migrations sequentially for deterministic behavior and to avoid + // creating many concurrent file reads/imports. + for (const filePath of filePaths) { + const actions = await sqlMigration(filePath); + migrationUnits.push({ + id: filePath, + filePaths: [filePath], + actions, + }); + } + return migrationUnits; }; @@ -149,7 +155,8 @@ export function createLegacySqlMigrationLoader(): MigrationLoader { /** * Creates a SQL migration loader that loads migrations from the file paths using the new SQL migration loading behaviour. - * While is handles the legacy format, it does add new behaviour that may be unwanted in existing usage so it has been separated from the legacy loader and can be used enabled as needed. + * While it handles the legacy format, it does add new behaviour that may be unwanted in existing usage so it has been + * separated from the legacy loader and can be used enabled as needed. * * @returns The SQL migration loader. */ @@ -223,6 +230,12 @@ function resolveMigrationLoader( /** * Associates the file paths to their extensions. + * + * - `Map` preserves insertion order, so extension buckets are iterated in + * the order their extension is first encountered in `filePaths`. + * - Within each extension bucket, we push in the order files appear in + * `filePaths`, so input order is preserved for that extension. + * * @param filePaths - The file paths to associate. * @returns The file paths associated to their extensions. */ @@ -375,12 +388,22 @@ function groupSqlFiles(filePaths: string[]): SqlGroup[] { return [...groups.values()]; } +/** + * Returns the group id for a SQL group. + * + * Compatibility: when using the new grouped SQL loader (`loader: "sql"`), + * ids are normalized so that switching representations doesn't double-track + * migrations. Concretely, `.up.sql` / `.down.sql` map to the equivalent + * `.sql` id (e.g. `001_init.up.sql` -> `001_init.sql`). + * + * @param group - The SQL file group to get the id for. + * @returns The group id. + */ function sqlGroupId(group: SqlGroup): string { const filePath = group.single ?? group.up ?? group.down; if (!filePath) { throw new Error(`No SQL file found for group ${group.id}`); } - return filePath.replace(/\.up\.sql$/, '.sql').replace(/\.down\.sql$/, '.sql'); } @@ -410,7 +433,7 @@ async function readSqlFileGroup(group: SqlGroup): Promise { }; } - const filePaths = [group.up, group.down, group.single].filter( + const filePaths = [group.single ?? group.up, group.down].filter( (p): p is string => Boolean(p) ); diff --git a/test/migration.spec.ts b/test/migration.spec.ts index c6cf57472..7e290ebec 100644 --- a/test/migration.spec.ts +++ b/test/migration.spec.ts @@ -9,7 +9,6 @@ import { getMigrationFilePaths, Migration, } from '../src/migration'; -import { getNumericPrefix } from '../src/utils'; const callbackMigration = '1414549381268_names.js'; const promiseMigration = '1414549381268_names_promise.js'; @@ -40,39 +39,6 @@ describe('migration', () => { dbMock.query = queryMock; }); - describe('getNumericPrefix', () => { - it('should allow any non-numeric character as a separator', () => { - expect(getNumericPrefix('1-line-as-separator.js')).toBe(1); - expect(getNumericPrefix('2_underscore-as-separator.ts')).toBe(2); - expect(getNumericPrefix('3 space-as-separator.sql')).toBe(3); - }); - - it('should fail with a non-numeric value', () => { - const prefix = 'invalid-prefix'; - expect(() => getNumericPrefix(prefix, logger)).toThrow( - new Error(`Cannot determine numeric prefix for "${prefix}"`) - ); - }); - - it('should get timestamp for normal timestamp', () => { - const now = Date.now(); - expect(getNumericPrefix(String(now), logger)).toBe(now); - }); - - it('should get timestamp for shortened iso format', () => { - const now = new Date(); - - expect( - getNumericPrefix(now.toISOString().replace(/\D/g, ''), logger) - ).toBe(now.valueOf()); - }); - - it('should get prefix for index strings', () => { - expect(getNumericPrefix('0001', logger)).toBe(1); - expect(getNumericPrefix('1234', logger)).toBe(1234); - }); - }); - describe('getMigrationFilePaths', () => { it('should resolve files directly in `dir`', async () => { const dir = 'test/migrations'; diff --git a/test/migrationLoader.spec.ts b/test/migrationLoader.spec.ts index 4d075a821..7f3a3fc54 100644 --- a/test/migrationLoader.spec.ts +++ b/test/migrationLoader.spec.ts @@ -3,6 +3,7 @@ import { mkdtemp, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { basename, join, resolve } from 'node:path'; import { describe, expect, it } from 'vitest'; +import { getMigrationFilePaths } from '../src/migration'; import type { MigrationLoader, MigrationLoaderConfig, @@ -139,6 +140,110 @@ describe('loadMigrationUnits', () => { }); }); + it('preserves input ordering within each extension bucket', async () => { + const calls: Array<{ ext: string; filePaths: string[] }> = []; + + const makeLoader = + (ext: string) => + async (bucketFilePaths: string[]) => { + calls.push({ ext, filePaths: [...bucketFilePaths] }); + + return bucketFilePaths.map((filePath) => ({ + id: filePath, + filePaths: [filePath], + actions: { up: () => {}, down: () => {}, shorthands: {} }, + })); + }; + + const config: MigrationLoaderConfig = { + migrationLoaderStrategies: [ + { extensions: ['.sql'], loader: makeLoader('.sql') }, + { extensions: ['.js'], loader: makeLoader('.js') }, + ], + }; + + // `.sql` appears first, so the `.sql` bucket must be processed first. + // Within each bucket, the order must match the input array order. + const filePaths = [ + join(process.cwd(), '010_one.sql'), + join(process.cwd(), '002_b.js'), + join(process.cwd(), '001_a.js'), + join(process.cwd(), '011_two.sql'), + ]; + + await loadMigrationUnits(config, filePaths); + + expect(calls).toHaveLength(2); + expect(calls[0]).toEqual({ + ext: '.sql', + filePaths: [filePaths[0], filePaths[3]], + }); + expect(calls[1]).toEqual({ + ext: '.js', + filePaths: [filePaths[1], filePaths[2]], + }); + }); + + it('orders 2_foo.js before 10_bar.js consistently between getMigrationFilePaths and loadMigrationUnits', async () => { + await withTempDir(async (dir) => { + const path2 = await writeMigrationFile( + dir, + '2_foo.js', + 'export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n' + ); + const path10 = await writeMigrationFile( + dir, + '10_bar.js', + 'export const shorthands = {};\nexport const up = () => {};\nexport const down = () => {};\n' + ); + + const filePaths = await getMigrationFilePaths(dir, {}); + + expect(filePaths.map((p) => basename(p))).toEqual(['2_foo.js', '10_bar.js']); + expect(filePaths).toEqual([path2, path10]); + + const customLoader: MigrationLoader = async (bucketFilePaths) => + bucketFilePaths.map((filePath) => ({ + id: filePath, + filePaths: [filePath], + actions: { up: () => {}, down: () => {}, shorthands: {} }, + })); + + const config: MigrationLoaderConfig = { + migrationLoaderStrategies: [{ extensions: ['.js'], loader: customLoader }], + }; + + const units = await loadMigrationUnits(config, filePaths); + + expect(units.map((u) => basename(u.filePaths[0]))).toEqual([ + '2_foo.js', + '10_bar.js', + ]); + }); + }); + + it('groups .up.sql-only (no .down.sql) into a single migration with undefined down action', async () => { + await withTempDir(async (dir) => { + const upPath = await writeMigrationFile( + dir, + '001_create_users.up.sql', + 'CREATE TABLE users(id serial primary key);\n' + ); + + const config: MigrationLoaderConfig = { + migrationLoaderStrategies: [{ extensions: ['.sql'], loader: 'sql' }], + }; + + const units = await loadMigrationUnits(config, [upPath]); + + expect(units).toHaveLength(1); + expect(units[0].id).toBe(join(dir, '001_create_users.sql')); + expect(units[0].filePaths).toEqual([upPath]); + expect(units[0].actions.up).toBeTypeOf('function'); + expect(units[0].actions.down).toBeUndefined(); + }); + }); + it('sorts cockroach files up to 062 in numeric order and keeps 062_view before 062_view_test', async () => { const customLoader: MigrationLoader = (filePaths) => Promise.resolve( diff --git a/test/utils/filenameComparators.spec.ts b/test/utils/comparators.spec.ts similarity index 77% rename from test/utils/filenameComparators.spec.ts rename to test/utils/comparators.spec.ts index a59d8801b..2cc1e5276 100644 --- a/test/utils/filenameComparators.spec.ts +++ b/test/utils/comparators.spec.ts @@ -3,21 +3,28 @@ import type { Logger } from '../../src/logger'; import { compareFileNamesByTimestamp, compareMigrationFileNames, - getSuffixFromFileName, + localeCompareStringsNumerically, } from '../../src/utils'; -describe('getSuffixFromFileName', () => { - it('extracts extension without the leading dot', () => { - expect(getSuffixFromFileName('001_migration.js')).toBe('js'); - expect(getSuffixFromFileName('001_migration.sql')).toBe('sql'); +describe('localeCompareStringsNumerically', () => { + it('sorts 062_view before 062_view_test', () => { + const files = ['062_view_test.js', '062_view.js']; + + const sorted = files.toSorted(localeCompareStringsNumerically); + + expect(sorted).toEqual(['062_view.js', '062_view_test.js']); }); - it('returns empty string when there is no extension', () => { - expect(getSuffixFromFileName('no-extension')).toBe(''); + it('sorts numeric substrings in numeric order', () => { + const files = ['10_bar.js', '2_foo.js']; + + const sorted = files.toSorted(localeCompareStringsNumerically); + + expect(sorted).toEqual(['2_foo.js', '10_bar.js']); }); - it('extracts only the last extension', () => { - expect(getSuffixFromFileName('archive.tar.gz')).toBe('gz'); + it('returns 0 for equal strings', () => { + expect(localeCompareStringsNumerically('abc', 'abc')).toBe(0); }); }); diff --git a/test/utils/fileNameUtils.spec.ts b/test/utils/fileNameUtils.spec.ts new file mode 100644 index 000000000..7b1e48e37 --- /dev/null +++ b/test/utils/fileNameUtils.spec.ts @@ -0,0 +1,64 @@ +import { describe, expect, it, vi } from 'vitest'; +import type { Logger } from '../../src/logger'; +import { + getNumericPrefix, + getSuffixFromFileName, +} from '../../src/utils'; + +describe('getSuffixFromFileName', () => { + it('extracts extension without the leading dot', () => { + expect(getSuffixFromFileName('001_migration.js')).toBe('js'); + expect(getSuffixFromFileName('001_migration.sql')).toBe('sql'); + }); + + it('returns empty string when there is no extension', () => { + expect(getSuffixFromFileName('no-extension')).toBe(''); + }); + + it('extracts only the last extension', () => { + expect(getSuffixFromFileName('archive.tar.gz')).toBe('gz'); + }); + + it('returns empty string for dotfiles without extension', () => { + expect(getSuffixFromFileName('.bashrc')).toBe(''); + }); +}); + +describe('getNumericPrefix', () => { + const logger: Logger = { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; + + it('should allow any non-numeric character as a separator', () => { + expect(getNumericPrefix('1-line-as-separator.js', logger)).toBe(1); + expect(getNumericPrefix('2_underscore-as-separator.ts', logger)).toBe(2); + expect(getNumericPrefix('3 space-as-separator.sql', logger)).toBe(3); + }); + + it('should fail with a non-numeric value', () => { + const prefix = 'invalid-prefix'; + expect(() => getNumericPrefix(prefix, logger)).toThrow( + new Error(`Cannot determine numeric prefix for "${prefix}"`) + ); + }); + + it('should get timestamp for normal timestamp', () => { + const now = Date.now(); + expect(getNumericPrefix(String(now), logger)).toBe(now); + }); + + it('should get timestamp for shortened iso format', () => { + const now = new Date(); + + expect( + getNumericPrefix(now.toISOString().replace(/\D/g, ''), logger) + ).toBe(now.valueOf()); + }); + + it('should get prefix for index strings', () => { + expect(getNumericPrefix('0001', logger)).toBe(1); + expect(getNumericPrefix('1234', logger)).toBe(1234); + }); +}); diff --git a/test/utils/stringComparison.spec.ts b/test/utils/stringComparison.spec.ts deleted file mode 100644 index 4babc28c4..000000000 --- a/test/utils/stringComparison.spec.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { localeCompareStringsNumerically } from '../../src/utils'; - -describe('localeCompareStringsNumerically', () => { - it('sorts 062_view before 062_view_test', () => { - const files = ['062_view_test.js', '062_view.js']; - - const sorted = files.toSorted(localeCompareStringsNumerically); - - expect(sorted).toEqual(['062_view.js', '062_view_test.js']); - }); -}); From 74614b713e44d1b109503f755c10e8e02c23338f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9tur=20=C3=9E=C3=B3r=20Valdimarsson?= Date: Fri, 20 Mar 2026 11:10:38 +0100 Subject: [PATCH 7/8] Preflight/Linting fixes. --- src/migrationLoader.ts | 11 +++++----- test/migrationLoader.spec.ts | 37 +++++++++++++++++++------------- test/utils/fileNameUtils.spec.ts | 11 ++++------ 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/migrationLoader.ts b/src/migrationLoader.ts index bf04ec31d..4a1c2a7fd 100644 --- a/src/migrationLoader.ts +++ b/src/migrationLoader.ts @@ -155,7 +155,7 @@ export function createLegacySqlMigrationLoader(): MigrationLoader { /** * Creates a SQL migration loader that loads migrations from the file paths using the new SQL migration loading behaviour. - * While it handles the legacy format, it does add new behaviour that may be unwanted in existing usage so it has been + * While it handles the legacy format, it does add new behaviour that may be unwanted in existing usage so it has been * separated from the legacy loader and can be used enabled as needed. * * @returns The SQL migration loader. @@ -230,12 +230,12 @@ function resolveMigrationLoader( /** * Associates the file paths to their extensions. - * + * * - `Map` preserves insertion order, so extension buckets are iterated in * the order their extension is first encountered in `filePaths`. * - Within each extension bucket, we push in the order files appear in * `filePaths`, so input order is preserved for that extension. - * + * * @param filePaths - The file paths to associate. * @returns The file paths associated to their extensions. */ @@ -390,12 +390,12 @@ function groupSqlFiles(filePaths: string[]): SqlGroup[] { /** * Returns the group id for a SQL group. - * + * * Compatibility: when using the new grouped SQL loader (`loader: "sql"`), * ids are normalized so that switching representations doesn't double-track * migrations. Concretely, `.up.sql` / `.down.sql` map to the equivalent * `.sql` id (e.g. `001_init.up.sql` -> `001_init.sql`). - * + * * @param group - The SQL file group to get the id for. * @returns The group id. */ @@ -404,6 +404,7 @@ function sqlGroupId(group: SqlGroup): string { if (!filePath) { throw new Error(`No SQL file found for group ${group.id}`); } + return filePath.replace(/\.up\.sql$/, '.sql').replace(/\.down\.sql$/, '.sql'); } diff --git a/test/migrationLoader.spec.ts b/test/migrationLoader.spec.ts index 7f3a3fc54..97567ae31 100644 --- a/test/migrationLoader.spec.ts +++ b/test/migrationLoader.spec.ts @@ -143,17 +143,17 @@ describe('loadMigrationUnits', () => { it('preserves input ordering within each extension bucket', async () => { const calls: Array<{ ext: string; filePaths: string[] }> = []; - const makeLoader = - (ext: string) => - async (bucketFilePaths: string[]) => { - calls.push({ ext, filePaths: [...bucketFilePaths] }); + const makeLoader = (ext: string) => (bucketFilePaths: string[]) => { + calls.push({ ext, filePaths: [...bucketFilePaths] }); - return bucketFilePaths.map((filePath) => ({ + return Promise.resolve( + bucketFilePaths.map((filePath) => ({ id: filePath, filePaths: [filePath], actions: { up: () => {}, down: () => {}, shorthands: {} }, - })); - }; + })) + ); + }; const config: MigrationLoaderConfig = { migrationLoaderStrategies: [ @@ -199,18 +199,25 @@ describe('loadMigrationUnits', () => { const filePaths = await getMigrationFilePaths(dir, {}); - expect(filePaths.map((p) => basename(p))).toEqual(['2_foo.js', '10_bar.js']); + expect(filePaths.map((p) => basename(p))).toEqual([ + '2_foo.js', + '10_bar.js', + ]); expect(filePaths).toEqual([path2, path10]); - const customLoader: MigrationLoader = async (bucketFilePaths) => - bucketFilePaths.map((filePath) => ({ - id: filePath, - filePaths: [filePath], - actions: { up: () => {}, down: () => {}, shorthands: {} }, - })); + const customLoader: MigrationLoader = (bucketFilePaths) => + Promise.resolve( + bucketFilePaths.map((filePath) => ({ + id: filePath, + filePaths: [filePath], + actions: { up: () => {}, down: () => {}, shorthands: {} }, + })) + ); const config: MigrationLoaderConfig = { - migrationLoaderStrategies: [{ extensions: ['.js'], loader: customLoader }], + migrationLoaderStrategies: [ + { extensions: ['.js'], loader: customLoader }, + ], }; const units = await loadMigrationUnits(config, filePaths); diff --git a/test/utils/fileNameUtils.spec.ts b/test/utils/fileNameUtils.spec.ts index 7b1e48e37..ad987d2bc 100644 --- a/test/utils/fileNameUtils.spec.ts +++ b/test/utils/fileNameUtils.spec.ts @@ -1,9 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; import type { Logger } from '../../src/logger'; -import { - getNumericPrefix, - getSuffixFromFileName, -} from '../../src/utils'; +import { getNumericPrefix, getSuffixFromFileName } from '../../src/utils'; describe('getSuffixFromFileName', () => { it('extracts extension without the leading dot', () => { @@ -52,9 +49,9 @@ describe('getNumericPrefix', () => { it('should get timestamp for shortened iso format', () => { const now = new Date(); - expect( - getNumericPrefix(now.toISOString().replace(/\D/g, ''), logger) - ).toBe(now.valueOf()); + expect(getNumericPrefix(now.toISOString().replace(/\D/g, ''), logger)).toBe( + now.valueOf() + ); }); it('should get prefix for index strings', () => { From 70a7d2d356829a48fa5f26b8ec73705ebfe060cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9tur=20=C3=9E=C3=B3r=20Valdimarsson?= Date: Fri, 20 Mar 2026 13:21:28 +0100 Subject: [PATCH 8/8] Post-review adjustments. No functional changes. --- bin/node-pg-migrate.ts | 1 - docs/src/api.md | 10 +++++----- src/migrationLoader.ts | 12 ++++++------ 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/bin/node-pg-migrate.ts b/bin/node-pg-migrate.ts index 5d98332c4..bf2777023 100755 --- a/bin/node-pg-migrate.ts +++ b/bin/node-pg-migrate.ts @@ -3,7 +3,6 @@ import type { DotenvConfigOptions } from 'dotenv'; // Import as node-pg-migrate, so tsup does not self-reference as '../dist' // otherwise this could not be imported by esm -// @ts-ignore: when a clean was made, the types are not present in the first run import type { RunnerOption } from 'node-pg-migrate'; import { jiti, diff --git a/docs/src/api.md b/docs/src/api.md index ebc20fb9a..bd9391097 100644 --- a/docs/src/api.md +++ b/docs/src/api.md @@ -38,7 +38,7 @@ which takes options argument with the following structure (similar to [command l ### MigrationLoaderStrategy -``` +```ts export interface MigrationLoaderStrategy { // File extensions handled by this strategy. extensions: string[]; @@ -49,14 +49,14 @@ export interface MigrationLoaderStrategy { * @param filePaths - The file paths to load migrations from. * @returns The migration units. */ - loader: MigrationLoader | "default" | "legacySql" | "sql"; + loader: MigrationLoader | 'default' | 'legacySql' | 'sql'; } ``` ### MigrationUnit -``` -export type MigrationUnit = { +```ts +export interface MigrationUnit { // The unique identifier for the migration unit. Represents the significant part of the file name used for tracking which migrations have been performed. id: string; @@ -65,5 +65,5 @@ export type MigrationUnit = { // The migration builder actions that are contained within the migration files. actions: MigrationBuilderActions; -}; +} ``` diff --git a/src/migrationLoader.ts b/src/migrationLoader.ts index 4a1c2a7fd..1ea20acbd 100644 --- a/src/migrationLoader.ts +++ b/src/migrationLoader.ts @@ -31,7 +31,7 @@ export const jiti = createJiti(process.cwd()); /** * A migration unit is a collection of file paths and related migration actions that are related to a single migration. */ -export type MigrationUnit = { +export interface MigrationUnit { /** * The unique identifier for the migration unit. Represents the significant part of the file name. * Used for tracking which migrations have been performed. @@ -47,7 +47,7 @@ export type MigrationUnit = { * The migration builder actions that are contained within the migration files. */ actions: MigrationBuilderActions; -}; +} /** * Loader function type. @@ -293,20 +293,20 @@ export async function loadMigrationUnits( * A parsed SQL file is a file that has been parsed and contains the id, direction and file path. * An intermediate step before the migration unit is created. */ -type ParsedSqlFile = { +interface ParsedSqlFile { id: string; direction: 'up' | 'down' | 'none'; filePath: string; -}; +} /** * A SQL group is a group of SQL files associated by significant part of the filename. */ -type SqlGroup = { +interface SqlGroup { id: string; up?: string; down?: string; single?: string; -}; +} /** * Parses a SQL file and returns the parsed file.