Skip to content

fix: circular import in SapDriver.ts #11750

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
michaelbromley merged 4 commits into from
Nov 10, 2025
Merged

fix: circular import in SapDriver.ts #11750

michaelbromley merged 4 commits into from
Nov 10, 2025

Conversation

@abendi
Copy link
Contributor

@abendi abendi commented Nov 3, 2025 •
edited by coderabbitai bot
Loading

Description of change

Deep-importing import { DataSource } from 'typeorm/data-source/DataSource.js' fails since 0.3.25 due to circular dependencies in SapDriver.ts. Runtime error looks like this:

/code/typeorm-import-issue/node_modules/typeorm/connection/Connection.js:12
class Connection extends DataSource_1.DataSource {
                                      ^

TypeError: Class extends value undefined is not a constructor or null
    at Object.<anonymous> (/code/typeorm-import-issue/node_modules/typeorm/connection/Connection.js:12:39)
    at Module._compile (node:internal/modules/cjs/loader:1730:14)
    at Object..js (node:internal/modules/cjs/loader:1895:10)
    at Module.load (node:internal/modules/cjs/loader:1465:32)
    at Function._load (node:internal/modules/cjs/loader:1282:12)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:235:24)
    at Module.require (node:internal/modules/cjs/loader:1487:12)
    at require (node:internal/modules/helpers:135:16)
    at Object.<anonymous> (/code/typeorm-import-issue/node_modules/typeorm/index.js:147:20)

Node.js v22.17.1

I have created a minimal reproduction example with some explanations: https://github.com/abendi/typeorm-circular-import.

Pull-Request Checklist

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

Summary by CodeRabbit

  • Chores
    • Minor internal module reorganization and import cleanup. No changes to behavior, APIs, error handling, or user-facing functionality. This is an internal maintenance update with no impact on workflows, UI, or user experience.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025 •
edited
Loading

Walkthrough

An import in src/driver/sap/SapDriver.ts was changed: ConnectionIsNotSetError was moved from a barrel import to an explicit path import. No behavioral or API changes.

Changes

Cohort / File(s) Change Summary
Import restructuring
src/driver/sap/SapDriver.ts		 Replaced barrel import of ConnectionIsNotSetError with an explicit import from ../../error/ConnectionIsNotSetError; no runtime or API changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Quick check: verify import path is correct and linter/build continue to resolve the module.

Suggested reviewers

  • gioboa
  • naorpeled
  • sgarner
  • mguida22

Poem

🐰 A tidy hop, an import found,
From barrel nest to clearer ground,
The code stays true, the flow unmoved,
A little change — neatly improved. ✨

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 directly and accurately describes the main change: fixing a circular import issue in SapDriver.ts by converting an inline import to an explicit import path.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • 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 9a43338 and 4b94b9f.

📒 Files selected for processing (1)
  • src/driver/sap/SapDriver.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/driver/sap/SapDriver.ts

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

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

pkuczynski
pkuczynski approved these changes Nov 7, 2025
View reviewed changes
Copy link
Collaborator

@pkuczynski pkuczynski left a comment

Choose a reason for hiding this comment

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

I am approving since this change is harmless, but bear in mind deep imports are strongly discouraged, especially same thing is exported from the main index file.

In the future paths might change and since the only officially supported interface for any package is the index file, this would be considered non breaking change, yet it would brake your code...

gioboa
gioboa approved these changes Nov 7, 2025
View reviewed changes
Copy link
Collaborator

@gioboa gioboa left a comment

Choose a reason for hiding this comment

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

Thanks for your help @abendi

@gioboa gioboa requested review from alumni and sgarner November 7, 2025 23:41
@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 7, 2025 •
edited
Loading

typeorm-sql-js-example

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

commit: 8b930f9

@coveralls
Copy link

coveralls commented Nov 8, 2025 •
edited
Loading

Coverage Status

coverage: 76.433%. remained the same
when pulling 8b930f9 on abendi:fix-circular-import
into 6381c8d on typeorm:master.

sgarner
sgarner approved these changes Nov 9, 2025
View reviewed changes
Copy link
Collaborator

@sgarner sgarner left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @abendi 💜

@michaelbromley michaelbromley merged commit bed7913 into typeorm:master Nov 10, 2025
62 checks passed
@abendi abendi deleted the fix-circular-import branch November 10, 2025 10:49
ThbltLmr pushed a commit to ThbltLmr/typeorm that referenced this pull request Dec 2, 2025
@abendi @ThbltLmr
fix: circular import in SapDriver.ts (typeorm#11750)
0f2dc3a
mgohin pushed a commit to mgohin/typeorm that referenced this pull request Jan 15, 2026
@abendi
fix: circular import in SapDriver.ts (typeorm#11750)
a711498
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pkuczynski pkuczynski pkuczynski approved these changes

@sgarner sgarner sgarner approved these changes

@michaelbromley michaelbromley michaelbromley approved these changes

@gioboa gioboa gioboa approved these changes

@alumni alumni Awaiting requested review from alumni

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@abendi @coveralls @pkuczynski @sgarner @michaelbromley @gioboa