CFSQL-1589 migrations_pattern for configuring recursive migration discovery #14089
Conversation
executeSql and the d1 execute command lower logger.loggerLevel to "error" when run with --json, so that human-readable logs don't end up in the JSON output. Previously the level was only restored on the happy path, so any early return or thrown error left the singleton logger muted — silencing later logger.warn/log calls (notably from migration helpers that wrap executeSql and are commonly mocked in tests). Wrap the log-level swap in try/finally so the level is always restored. Also stop using a literal /tmp/my-migrations-go-here path in two migrate tests; they were leaking state between runs and depending on the muted-logger bug above. They now build the migrations dir under the per-test runInTempDir() cwd.
🦋 Changeset detectedLatest commit: f9d669e The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
✅ All changesets look good |
|
UnknownError: ProviderInitError |
|
@alsuren Bonk workflow failed. Check the logs for details. View workflow run · To retry, trigger Bonk again. |
5b8ccb5 to
bd8db0b
Compare
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
@cloudflare/wrangler-bundler
commit: |
Adds a `migrations_pattern` config field to D1 bindings so users can
opt their D1 migrations into a nested layout (drizzle's
`migrations/*/migration.sql`, prisma-style folders, custom
extensions, …) instead of the implicit `migrations/*.sql`. Without
`migrations_pattern`, behaviour is unchanged.
Rules
-----
When `migrations_pattern` is set:
- `migrations_dir` must also be set explicitly (no implicit default).
- The pattern must literally start with `${migrations_dir}/`. This
keeps the relationship between the two settings obvious and makes
"name relative to migrations_dir" deterministic via prefix-strip.
Names written to the `d1_migrations` table are always relative to
`migrations_dir` (with forward-slash separators), so the table stays
portable across machines regardless of how the user wrote their
config.
Implementation
--------------
- `resolveMigrationsConfig({ databaseInfo, configPath })` returns
a single `MigrationsConfig` object with every field defaulted
and normalized. Called once per command (`apply`, `list`,
`create`) and threaded through the helpers. "Parse, don't
validate" — the only way to get a `MigrationsConfig` is via the
resolver, and a `MigrationsConfig` is by construction valid.
- `getMigrationNames` walks `projectPath/migrationsDir` with a
`Minimatch` for the pattern. Uses minimatch's `partial: true`
mode to prune subdirectory descents (so a `*.sql` pattern never
recurses, `*/migration.sql` only descends one level, `**/*.sql`
recurses unconditionally — patterns opt into walking everything).
This keeps the cost bounded even if a user has a
`migrations/node_modules` sitting around.
- `compareMigrationPaths` is the named ordering: numeric on the
first path segment's leading integer, lex tiebreaker on the full
path, numbered files before unnumbered. Preserves the old
inconsistent-padding behaviour (`1_a, 9_b, 10_c` still sort
numerically).
- `getNextMigrationNumber` delegates to `getMigrationNames`, so
it respects `migrations_pattern` and counts numbered directories
(drizzle-style) alongside numbered files.
- When `migrations_pattern` matches nothing but
`${migrations_dir}/*/migration.sql` (drizzle's layout) matches
files on disk, the command logs a hint pointing the user at the
right pattern.
- `wrangler d1 migrations create` rejects with an actionable error
before touching the filesystem if the file it would write
wouldn't match `migrations_pattern` (the migrations dir is no
longer created as a side effect of a failing command).
Tests
-----
- 49 unit tests in `helpers.test.ts` covering walk, pattern
matching, ordering, drizzle-hint output, prune behaviour,
absolute paths (POSIX + Windows-only), and the
`resolveMigrationsConfig` rejection paths.
- 25 end-to-end tests in `migrate.test.ts` covering all three
commands' interaction with `migrations_pattern`, including
inline-snapshot tests of every user-facing error and warning.
- Tests live in the describe block matching their subcommand
(`create` / `apply` / `list`), not a separate
`migrations_pattern` block.
CFSQL-1589
bd8db0b to
9bdc7ba
Compare
|
Review posted successfully. Here's a summary of what I found: PR #14089 adds One actionable issue found: |
Bonk suggestion Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
|
It was not easy to review, but it's absolutely not your fault. Thanks for taking the time to write it - I really like the direction. |
Adds documentation for the new `migrations_pattern` D1 binding config field landing in wrangler PR cloudflare/workers-sdk#14089. Updates: - `d1/reference/migrations.mdx` \u2014 `migrations_pattern` shown in the example config, plus a new "Nested migration layouts" section that explains the default behaviour, shows the Drizzle-style nested example, lists the rules (`migrations_dir` required, pattern must start with `${migrations_dir}/`, recorded names are relative to `migrations_dir`), and notes that `wrangler d1 migrations create` only writes top-level files so ORM users should use their ORM's generate command. - `workers/wrangler/configuration.mdx` \u2014 `migrations_pattern` bullet next to the existing `migrations_dir` bullet in the D1 bindings reference.
|
Codeowners approval required for this PR:
Show detailed file reviewers |
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
Adds documentation for the new `migrations_pattern` D1 binding config field landing in wrangler PR cloudflare/workers-sdk#14089. Updates: - `d1/reference/migrations.mdx` \u2014 `migrations_pattern` shown in the example config, plus a new "Nested migration layouts" section that explains the default behaviour, shows the Drizzle-style nested example, lists the rules (`migrations_dir` required, pattern must start with `${migrations_dir}/`, recorded names are relative to `migrations_dir`), and notes that `wrangler d1 migrations create` only writes top-level files so ORM users should use their ORM's generate command. - `workers/wrangler/configuration.mdx` \u2014 `migrations_pattern` bullet next to the existing `migrations_dir` bullet in the D1 bindings reference.
Adds documentation for the new `migrations_pattern` D1 binding config field landing in wrangler PR cloudflare/workers-sdk#14089. Updates: - `d1/reference/migrations.mdx` \u2014 `migrations_pattern` shown in the example config, plus a new "Nested migration layouts" section that explains the default behaviour, shows the Drizzle-style nested example, lists the rules (`migrations_dir` required, pattern must start with `${migrations_dir}/`, recorded names are relative to `migrations_dir`), and notes that `wrangler d1 migrations create` only writes top-level files so ORM users should use their ORM's generate command. - `workers/wrangler/configuration.mdx` \u2014 `migrations_pattern` bullet next to the existing `migrations_dir` bullet in the D1 bindings reference.
Fixes #13257 (CFSQL-1589).
Adds a
migrations_patternconfig field to D1 bindings so users canopt their D1 migrations into a nested layout (drizzle's
migrations/*/migration.sql, prisma-style folders, customextensions, …) instead of the implicit
migrations/*.sql. Withoutmigrations_pattern, behaviour is unchanged.Also includes a commit to fix our logging, because it was causing a bunch of test failures as I added more tests (best reviewed with whitespace hidden: 54d293f?w=1 .
A picture of a cute animal (not mandatory, but encouraged)
-- https://www.asciiart.eu/art/64248696ab474307