Skip to content

feat: adds glob support for dir and ignorePattern#1274

Merged
Shinigami92 merged 10 commits into
salsita:mainfrom
benkroeger:feat/add_glob_support
Sep 23, 2024
Merged

feat: adds glob support for dir and ignorePattern#1274
Shinigami92 merged 10 commits into
salsita:mainfrom
benkroeger:feat/add_glob_support

Conversation

@benkroeger

@benkroeger benkroeger commented Sep 18, 2024

Copy link
Copy Markdown
Contributor

adds support to work with glob patterns for dir and ignorePattern as alternative to the current implementation (which uses dir as relative path to cwd() and generates a regex from ignorePattern.

CLI requires to additionally set the flag --use-glob while the RunnerOptions for API have been extended with a useGlob boolean property.

When using glob, migration files are sorted by their absolute path (via String.localeCompare) while setting numeric: true in the Intl.Collator().
The original sorting has not changed (still attempts to extract Date from the filename (either utc or timestamp) and sorts by the date's numeric value if possible - localeCompare otherwise)

This also fixes the most parts (if not all) of #932 and thus makes #1226 obsolete
Seems to make #911 obsolete as well

@Shinigami92 Shinigami92 added c: feature Request for new feature p: 1-normal Nothing urgent labels Sep 18, 2024
@Shinigami92

Copy link
Copy Markdown
Collaborator

looks like you were working from your old main branch 🙂
please update the branch and resolve the conflict

Signed-off-by: Benjamin Kroeger <benjamin.kroeger@gmail.com>
@benkroeger
benkroeger force-pushed the feat/add_glob_support branch from d14c344 to 42f98e6 Compare September 18, 2024 12:03
@github-actions

github-actions Bot commented Sep 18, 2024

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🟢 Lines 92.02% (🎯 90%)
⬇️ -0.01%
3196 / 3473
🟢 Statements 92.02% (🎯 90%)
⬇️ -0.01%
3196 / 3473
🟢 Functions 95.88% (🎯 90%)
⬆️ +0.04%
256 / 267
🟢 Branches 90.32% (🎯 85%)
⬆️ +0.29%
822 / 910
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Unchanged Files
src/db.ts 84.29% 89.28% 87.5% 84.29% 75-77, 107, 110-119, 121-122, 162-164
src/index.ts 100% 100% 100% 100%
src/migration.ts 72.9% 86.79% 66.66% 72.9% 133-136, 157-159, 161-173, 212-218, 223-227, 230, 232-236, 238-245, 248, 251-256, 258-259, 316-317, 343-345, 353-354, 371-374, 403-404
src/migrationBuilder.ts 95.43% 92.85% 88.88% 95.43% 354-358, 497-499, 501-502, 526-527
src/runner.ts 75.43% 59.25% 90% 75.43% 45, 65-66, 75-76, 125-126, 168-171, 180-184, 197, 201-202, 204-206, 208-214, 233, 235-241, 244, 257, 269, 271-277, 279-282, 285-288, 298-299, 308-310, 319, 321, 323-330
src/sqlMigration.ts 90% 100% 80% 90% 45-46, 48-49
src/types.ts 100% 100% 100% 100%
src/operations/sql.ts 100% 100% 100% 100%
src/operations/casts/createCast.ts 100% 100% 100% 100%
src/operations/casts/dropCast.ts 100% 100% 100% 100%
src/operations/casts/index.ts 100% 100% 100% 100%
src/operations/domains/alterDomain.ts 100% 100% 100% 100%
src/operations/domains/createDomain.ts 100% 100% 100% 100%
src/operations/domains/dropDomain.ts 100% 100% 100% 100%
src/operations/domains/index.ts 100% 100% 100% 100%
src/operations/domains/renameDomain.ts 100% 100% 100% 100%
src/operations/domains/shared.ts 100% 100% 100% 100%
src/operations/extensions/createExtension.ts 100% 100% 100% 100%
src/operations/extensions/dropExtension.ts 100% 100% 100% 100%
src/operations/extensions/index.ts 100% 100% 100% 100%
src/operations/extensions/shared.ts 100% 100% 100% 100%
src/operations/functions/createFunction.ts 95.58% 94.44% 100% 95.58% 71-73
src/operations/functions/dropFunction.ts 100% 100% 100% 100%
src/operations/functions/index.ts 100% 100% 100% 100%
src/operations/functions/renameFunction.ts 100% 100% 100% 100%
src/operations/functions/shared.ts 100% 100% 100% 100%
src/operations/grants/grantOnSchemas.ts 100% 100% 100% 100%
src/operations/grants/grantOnTables.ts 100% 100% 100% 100%
src/operations/grants/grantRoles.ts 100% 100% 100% 100%
src/operations/grants/index.ts 100% 100% 100% 100%
src/operations/grants/revokeOnSchemas.ts 100% 100% 100% 100%
src/operations/grants/revokeOnTables.ts 100% 100% 100% 100%
src/operations/grants/revokeRoles.ts 100% 100% 100% 100%
src/operations/grants/shared.ts 95.65% 85.71% 100% 95.65% 71
src/operations/indexes/createIndex.ts 96.29% 95.23% 100% 96.29% 74-75
src/operations/indexes/dropIndex.ts 100% 100% 100% 100%
src/operations/indexes/index.ts 100% 100% 100% 100%
src/operations/indexes/shared.ts 88% 86.95% 100% 88% 22, 32-35, 47
src/operations/materializedViews/alterMaterializedView.ts 100% 100% 100% 100%
src/operations/materializedViews/createMaterializedView.ts 100% 100% 100% 100%
src/operations/materializedViews/dropMaterializedView.ts 100% 100% 100% 100%
src/operations/materializedViews/index.ts 100% 100% 100% 100%
src/operations/materializedViews/refreshMaterializedView.ts 100% 100% 100% 100%
src/operations/materializedViews/renameMaterializedView.ts 100% 100% 100% 100%
src/operations/materializedViews/renameMaterializedViewColumn.ts 100% 100% 100% 100%
src/operations/materializedViews/shared.ts 100% 87.5% 100% 100%
src/operations/operators/addToOperatorFamily.ts 100% 100% 100% 100%
src/operations/operators/createOperator.ts 100% 100% 100% 100%
src/operations/operators/createOperatorClass.ts 100% 83.33% 100% 100%
src/operations/operators/createOperatorFamily.ts 100% 100% 100% 100%
src/operations/operators/dropOperator.ts 100% 100% 100% 100%
src/operations/operators/dropOperatorClass.ts 100% 100% 100% 100%
src/operations/operators/dropOperatorFamily.ts 100% 100% 100% 100%
src/operations/operators/index.ts 100% 100% 100% 100%
src/operations/operators/removeFromOperatorFamily.ts 100% 100% 100% 100%
src/operations/operators/renameOperatorClass.ts 100% 100% 100% 100%
src/operations/operators/renameOperatorFamily.ts 100% 100% 100% 100%
src/operations/operators/shared.ts 85.71% 75% 100% 85.71% 24-25, 38
src/operations/policies/alterPolicy.ts 100% 100% 100% 100%
src/operations/policies/createPolicy.ts 100% 100% 100% 100%
src/operations/policies/dropPolicy.ts 100% 100% 100% 100%
src/operations/policies/index.ts 100% 100% 100% 100%
src/operations/policies/renamePolicy.ts 100% 100% 100% 100%
src/operations/policies/shared.ts 100% 100% 100% 100%
src/operations/roles/alterRole.ts 100% 100% 100% 100%
src/operations/roles/createRole.ts 100% 75% 100% 100%
src/operations/roles/dropRole.ts 100% 100% 100% 100%
src/operations/roles/index.ts 100% 100% 100% 100%
src/operations/roles/renameRole.ts 100% 100% 100% 100%
src/operations/roles/shared.ts 98.07% 76.92% 100% 98.07% 78
src/operations/schemas/createSchema.ts 100% 100% 100% 100%
src/operations/schemas/dropSchema.ts 100% 100% 100% 100%
src/operations/schemas/index.ts 100% 100% 100% 100%
src/operations/schemas/renameSchema.ts 100% 100% 100% 100%
src/operations/sequences/alterSequence.ts 94.11% 75% 100% 94.11% 23
src/operations/sequences/createSequence.ts 100% 100% 100% 100%
src/operations/sequences/dropSequence.ts 100% 100% 100% 100%
src/operations/sequences/index.ts 100% 100% 100% 100%
src/operations/sequences/renameSequence.ts 100% 100% 100% 100%
src/operations/sequences/shared.ts 78.57% 68.75% 100% 78.57% 41, 43-44, 49-50, 63-64, 69-70
src/operations/tables/addColumns.ts 100% 80% 100% 100%
src/operations/tables/addConstraint.ts 100% 100% 100% 100%
src/operations/tables/alterColumn.ts 88.57% 76.47% 100% 88.57% 75, 82-85, 87-89
src/operations/tables/alterTable.ts 100% 100% 100% 100%
src/operations/tables/createTable.ts 86.2% 66.66% 100% 86.2% 44-48, 65, 75-76
src/operations/tables/dropColumns.ts 100% 100% 100% 100%
src/operations/tables/dropConstraint.ts 100% 100% 100% 100%
src/operations/tables/dropTable.ts 100% 100% 100% 100%
src/operations/tables/index.ts 100% 100% 100% 100%
src/operations/tables/renameColumn.ts 100% 100% 100% 100%
src/operations/tables/renameConstraint.ts 100% 100% 100% 100%
src/operations/tables/renameTable.ts 100% 100% 100% 100%
src/operations/tables/shared.ts 82.31% 78.46% 80% 82.31% 137-138, 141-142, 195-199, 224, 228-229, 248-249, 274-275, 278-285, 288-289, 426-431, 433-436, 438-448, 450-451
src/operations/triggers/createTrigger.ts 86.41% 68.18% 100% 86.41% 53-54, 66-67, 70-71, 74-77, 113
src/operations/triggers/dropTrigger.ts 100% 100% 100% 100%
src/operations/triggers/index.ts 100% 100% 100% 100%
src/operations/triggers/renameTrigger.ts 100% 100% 100% 100%
src/operations/triggers/shared.ts 100% 100% 100% 100%
src/operations/types/addTypeAttribute.ts 100% 100% 100% 100%
src/operations/types/addTypeValue.ts 100% 100% 100% 100%
src/operations/types/createType.ts 100% 100% 100% 100%
src/operations/types/dropType.ts 100% 100% 100% 100%
src/operations/types/dropTypeAttribute.ts 100% 100% 100% 100%
src/operations/types/index.ts 100% 100% 100% 100%
src/operations/types/renameType.ts 100% 100% 100% 100%
src/operations/types/renameTypeAttribute.ts 100% 100% 100% 100%
src/operations/types/renameTypeValue.ts 100% 100% 100% 100%
src/operations/types/setTypeAttribute.ts 100% 100% 100% 100%
src/operations/views/alterView.ts 100% 100% 100% 100%
src/operations/views/alterViewColumn.ts 100% 100% 100% 100%
src/operations/views/createView.ts 100% 100% 100% 100%
src/operations/views/dropView.ts 100% 100% 100% 100%
src/operations/views/index.ts 100% 100% 100% 100%
src/operations/views/renameView.ts 100% 100% 100% 100%
src/operations/views/shared.ts 100% 66.66% 100% 100%
src/utils/PgLiteral.ts 88.88% 100% 75% 88.88% 37-38
src/utils/StringIdGenerator.ts 100% 100% 100% 100%
src/utils/createSchemalize.ts 100% 100% 100% 100%
src/utils/createTransformer.ts 100% 100% 100% 100%
src/utils/decamelize.ts 100% 100% 100% 100%
src/utils/escapeValue.ts 100% 100% 100% 100%
src/utils/formatLines.ts 100% 100% 100% 100%
src/utils/formatParams.ts 100% 100% 100% 100%
src/utils/getMigrationTableSchema.ts 100% 100% 100% 100%
src/utils/getSchemas.ts 100% 100% 100% 100%
src/utils/identity.ts 100% 100% 100% 100%
src/utils/index.ts 100% 100% 100% 100%
src/utils/intersection.ts 100% 100% 100% 100%
src/utils/makeComment.ts 100% 100% 100% 100%
src/utils/quote.ts 100% 100% 100% 100%
src/utils/toArray.ts 100% 100% 100% 100%
src/utils/types.ts 100% 100% 100% 100%
Generated in workflow #1172

Comment thread bin/node-pg-migrate.ts Outdated
Comment thread docs/src/api.md Outdated
@Shinigami92
Shinigami92 marked this pull request as draft September 18, 2024 12:05
@Shinigami92

Copy link
Copy Markdown
Collaborator

Set to draft for now, because some failing tests
Looks like you are already a trusted contributor, so the tests will run automatically 🙂

I gave a really simple first review, but I need to have a look into details, but assume I wont have time before upcoming Saturday, because I will be on a conference

But it looks really promising and potentially even like a better solution that the other strategy ❤️

Signed-off-by: Benjamin Kroeger <benjamin.kroeger@gmail.com>
Signed-off-by: Benjamin Kroeger <benjamin.kroeger@gmail.com>
because the previous implementation used that only on the migration file's basename (which also caused two files with same name but different extensions to be treated as equal priority)

Signed-off-by: Benjamin Kroeger <benjamin.kroeger@gmail.com>
Signed-off-by: Benjamin Kroeger <benjamin.kroeger@gmail.com>
@benkroeger
benkroeger marked this pull request as ready for review September 18, 2024 13:19
Comment thread src/migration.ts Outdated
Comment thread src/migration.ts Outdated
Comment thread src/migration.ts Outdated
Comment thread src/types.ts Outdated
Comment thread src/migration.ts Outdated
Co-authored-by: Shinigami <chrissi92@hotmail.de>
Signed-off-by: Benjamin Kroeger <benjamin.kroeger@gmail.com>

@Shinigami92 Shinigami92 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I had an in depth look and really like the PR 👍
As you already wrote in the ? comments, we are free to change the name or inner structure of these function. Theoretically users could import them because it is exported in package.json, but I would count this more as a legacy backward compatibility and not official support. So I'm free to break even in a minor version bump.

But please you now need to change the both sections of ! and ? comments so they are solved in this PR, before getting merged.
And if you like, you can also directly use the options destructure pattern in this PR, or I could do this in a separate PR later on.

also updates code documentation

Signed-off-by: Benjamin Kroeger <benjamin.kroeger@gmail.com>
Comment thread src/migration.ts Outdated
Comment thread src/migration.ts Outdated
@theoephraim

Copy link
Copy Markdown
Contributor

It may not matter so much in this case, but I recently had to implement some globbing behaviour and found fdir (https://www.npmjs.com/package/fdir) to be much more performant. Should be a simple swap.

@Shinigami92

Copy link
Copy Markdown
Collaborator

It may not matter so much in this case, but I recently had to implement some globbing behaviour and found fdir (npmjs.com/package/fdir) to be much more performant. Should be a simple swap.

image

... Lets stay with glob for now, I plan anyways to use glob from node:fs when we are on Node v22 https://nodejs.org/api/fs.html#fsglobpattern-options-callback

@benkroeger

Copy link
Copy Markdown
Contributor Author

It may not matter so much in this case, but I recently had to implement some globbing behaviour and found fdir (https://www.npmjs.com/package/fdir) to be much more performant. Should be a simple swap.

if you're going with pure js implementations, there is no faster lib that adheres the de-facto-spec as good as this one.
Also see Comparison to Other JavaScript Glob Implementations.

I don't think that performance is that much of the essence here either.

Signed-off-by: Benjamin Kroeger <benjamin.kroeger@gmail.com>
@Shinigami92
Shinigami92 merged commit 6d4570c into salsita:main Sep 23, 2024
@Shinigami92

Copy link
Copy Markdown
Collaborator

Might release next few days, I also currently await feedback of #1262

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: feature Request for new feature p: 1-normal Nothing urgent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants