fix: getPendingMigrations unnecessarily creating migrations table

[pkuczynski]

Description of change

Fixes #10858

Pull-Request Checklist

• [x] Code is up-to-date with the master branch
• [x] This pull request links relevant issues as Fixes #00000
• [ ] There are new or updated unit tests validating the change
• [ ] Documentation has been updated to reflect this change

Summary by CodeRabbit

• Bug Fixes

  • Improved resilience when the migrations table is missing; operations now proceed without errors and return sensible defaults.
  • Corrected pending migration detection so the app correctly reports pending migrations when none have been executed.

• Refactor

  • Streamlined migration execution flow to avoid unnecessary work and simplify logic, improving reliability and maintainability.

[pkg-pr-new[bot]]

• typeorm-sql-js-example

  npm i https://pkg.pr.new/typeorm/typeorm@11672
  

commit: f888670

[pkuczynski]

I was looking for a way of providing tests for this change, but I could not find any viable example. If you point me to one, I am happy to extend the scope of this PR.

There was no need to update documentation.

[coveralls]

coverage: 80.769% (-0.004%) from 80.773% when pulling f888670e96f5326a9947e8973b7b8a3acaee0269 on pkuczynski:fix/getPendingMigrations into 2d8c5158db1aef458cd909db05059fff9129305a on typeorm:master.

[coderabbitai[bot]]

Walkthrough

Removes the public getAllMigrations method. Updates getExecutedMigrations to add a MongoDB-specific comment and, for non-MongoDB drivers, call queryRunner.hasTable(this.migrationsTable) and return [] if the table is missing before loading executed migrations. Updates getPendingMigrations to use this.getMigrations() and to return all migrations early when no executed migrations are found.

Changes

Cohort / File(s)	Change Summary
Migration executor refactor src/migration/MigrationExecutor.ts	Removed public getAllMigrations(). In getExecutedMigrations, added a MongoDB-specific comment and, for non-MongoDB drivers, a queryRunner.hasTable(this.migrationsTable) check with an early [] return if the table doesn't exist. In getPendingMigrations, replaced await getAllMigrations() with this.getMigrations() and added an early return of allMigrations when no executed migrations exist.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Caller (App/CLI)
  participant ME as MigrationExecutor
  participant QR as QueryRunner
  participant DB as Database

  Note over ME: getExecutedMigrations()
  C->>ME: getExecutedMigrations()
  ME->>QR: hasTable(migrationsTable)  %% skipped/DB-specific for MongoDB
  QR->>DB: Check table existence
  DB-->>QR: true/false
  alt Table missing (non-MongoDB)
    QR-->>ME: false
    ME-->>C: []
  else Table exists
    QR-->>ME: true
    ME->>QR: load executed migrations
    QR-->>ME: executed list
    ME-->>C: executed list
  end

  Note over ME: getPendingMigrations()
  C->>ME: getPendingMigrations()
  ME->>ME: allMigrations = this.getMigrations()
  ME->>ME: executed = getExecutedMigrations()
  alt No executed
    ME-->>C: allMigrations
  else Some executed
    ME->>ME: compute difference (pending)
    ME-->>C: pending list
  end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

• OSA413

Poem

I thump my paws at schema’s gate,
A Mongo note, a wary wait—
If table’s gone, I skip the track;
If present, I hop knowledge back.
Pending carrots neatly queued, I prance—🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly identifies that this change fixes the issue where getPendingMigrations was creating the migrations table unnecessarily, which directly reflects the main objective of the pull request. It is specific to the affected method and avoids generic language, making it clear to reviewers what is being addressed.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• [ ] 📝 Generate Docstrings

🧪 Generate unit tests

• [ ] Create PR with unit tests
• [ ] Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f4761ed575a77c2add1662300b41716c42a3ca58 and 17e7b8747cb8f94e8b530744555acf119329911e.

📒 Files selected for processing (1)

• src/migration/MigrationExecutor.ts (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/migration/MigrationExecutor.ts (1)

src/migration/Migration.ts (1)

• Migration (6-54)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)

• GitHub Check: tests-linux (20) / mysql_mariadb
• GitHub Check: tests-linux (18) / better-sqlite3
• GitHub Check: tests-linux (20) / mssql
• GitHub Check: tests-linux (20) / sqljs
• GitHub Check: tests-linux (20) / better-sqlite3
• GitHub Check: tests-linux (20) / cockroachdb
• GitHub Check: tests-linux (20) / sqlite
• GitHub Check: tests-linux (18) / sap
• GitHub Check: tests-linux (18) / postgres (17)
• GitHub Check: tests-linux (18) / oracle
• GitHub Check: tests-linux (18) / postgres (14)
• GitHub Check: tests-linux (18) / mssql
• GitHub Check: tests-linux (18) / sqlite
• GitHub Check: tests-linux (18) / mysql_mariadb_latest
• GitHub Check: tests-linux (18) / mongodb
• GitHub Check: tests-linux (18) / mysql_mariadb
• GitHub Check: tests-windows / sqlite
• GitHub Check: tests-windows / sqljs
• GitHub Check: tests-windows / better-sqlite3
• GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (2)

src/migration/MigrationExecutor.ts (2)

97-103: Mongo guard keeps getExecutedMigrations compatible

Nice touch skipping hasTable on Mongo—this keeps the method from throwing NotSupportedError while still short-circuiting for other drivers when the table is absent.

---

113-117: Efficient handling when nothing ran yet

Using the synchronous getMigrations() and short-circuiting when no executions exist keeps the path side-effect free and avoids extra filtering work.

[!TIP]

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[qodo-free-for-open-source-projects[bot]]

PR Code Suggestions ✨

Latest suggestions up to f888670

Category	Suggestion	Impact
General	Use type-safe MongoDB check Refactor the MongoDB driver check to use the InstanceChecker.isMongoQueryRunner utility instead of a string comparison for improved type safety and code consistency. src/migration/MigrationExecutor.ts [104-116] public async getExecutedMigrations(): Promise<Migration[]> { return this.withQueryRunner(async (queryRunner) => { // There is no need to check if migrations table exists for MongoDB, // as it's handled in loadExecutedMigrations - if (this.connection.driver.options.type !== "mongodb") { + if (!InstanceChecker.isMongoQueryRunner(queryRunner)) { const exist = await queryRunner.hasTable(this.migrationsTable) if (!exist) return [] } return await this.loadExecutedMigrations(queryRunner) }) } [ ] Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly proposes using InstanceChecker for a more robust and type-safe check of the MongoDB driver, which is better practice and more consistent with the project's coding style than comparing driver type strings.	Low
[ ] More

---

Previous suggestions

Suggestions

Category	Suggestion	Impact
General	Use type-safe instance check Replace the string-based check for the MongoDB driver with a type-safe check using InstanceChecker.isMongoQueryRunner(queryRunner) for improved robustness. src/migration/MigrationExecutor.ts [108-112] -if (this.connection.driver.options.type !== "mongodb") { +if (!InstanceChecker.isMongoQueryRunner(queryRunner)) { const exist = await queryRunner.hasTable(this.migrationsTable) if (!exist) return [] } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly proposes replacing a string-based driver check with InstanceChecker, which is a more robust, type-safe, and maintainable approach that aligns with the project's coding patterns.	Low

[qodo-free-for-open-source-projects[bot]]

PR Code Suggestions ✨

Category	Suggestion	Impact
General	Use type-safe instance check Replace the string-based check for the MongoDB driver with a type-safe check using InstanceChecker.isMongoQueryRunner(queryRunner) for improved robustness. src/migration/MigrationExecutor.ts [108-112] -if (this.connection.driver.options.type !== "mongodb") { +if (!InstanceChecker.isMongoQueryRunner(queryRunner)) { const exist = await queryRunner.hasTable(this.migrationsTable) if (!exist) return [] } [ ] Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly proposes replacing a string-based driver check with InstanceChecker, which is a more robust, type-safe, and maintainable approach that aligns with the project's coding patterns.	Low
[ ] More

[OSA413]

At least some manual testing is required, I agree with gioboa. I will be able to check that on the weekend.