Skip to content

fix: remove circular dependency in packages/prisma#24596

Merged
volnei merged 2 commits intomainfrom
devin/1761046816-fix-circular-dependency-prisma
Oct 21, 2025
Merged

fix: remove circular dependency in packages/prisma#24596
volnei merged 2 commits intomainfrom
devin/1761046816-fix-circular-dependency-prisma

Conversation

@volnei
Copy link
Copy Markdown
Contributor

@volnei volnei commented Oct 21, 2025

What does this PR do?

Fixes a circular dependency in packages/prisma that was detected using madge.

Circular dependency chain (before):

packages/lib/server/repository/deployment.ts 
  → packages/prisma/index.ts 
    → packages/prisma/extensions/usage-tracking.ts 
      → packages/lib/server/repository/deployment.ts (circular!)

Solution:

  • Created an inline InlineDeploymentRepository class within usage-tracking.ts that implements the IDeploymentRepository interface
  • Removed the import of DeploymentRepository which was causing the circular dependency
  • The inline implementation has identical logic to the original repository

Verification:

  • Ran npx madge --circular --extensions ts,tsx packages/prisma before: 1 circular dependency found
  • Ran npx madge --circular --extensions ts,tsx packages/prisma after: ✔ No circular dependency found!

Link to Devin run: https://app.devin.ai/sessions/1c2e0e210b464a5d877af6b3faf88bd4
Requested by: Volnei Munhoz (@volnei)

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox. N/A - This is an internal refactoring that doesn't change external behavior or APIs.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works. Note: Existing tests should continue to pass as this doesn't change behavior, but no new tests were added for this specific refactoring.

How should this be tested?

This is a refactoring that doesn't change runtime behavior. Testing should focus on:

  1. Verify no circular dependencies: Run npx madge --circular --extensions ts,tsx packages/prisma
  2. Run existing tests: Ensure all existing tests still pass, especially those related to usage tracking and license key validation
  3. Manual verification (if applicable): Create a booking or user to trigger usage tracking and verify the license key service still works correctly

Human Review Checklist

⚠️ Important items for reviewers:

  1. Code duplication concern: The InlineDeploymentRepository duplicates logic from DeploymentRepository. If DeploymentRepository is ever updated, this inline version must be updated too. Consider adding a comment or documentation about this coupling.

  2. Verify identical behavior: Compare InlineDeploymentRepository methods with DeploymentRepository to confirm they're functionally identical:

    • getLicenseKeyWithId() - same query, same return
    • getSignatureToken() - same query, same return
  3. Consider alternative solutions: Is inlining the best approach, or should we explore other architectural patterns to break this circular dependency?

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas (N/A for this simple refactoring)
  • I have checked if my changes generate no new warnings (lint passed)

Summary by cubic

Removed a circular dependency in packages/prisma by inlining the deployment repository in usage-tracking.ts. The loop between deployment.ts, prisma/index.ts, and usage-tracking.ts is gone, with no behavior change.

  • Bug Fixes
    • Inlined IDeploymentRepository implementation in usage-tracking.ts and removed DeploymentRepository import.
    • Verified with madge: no circular dependencies.

- Replaced DeploymentRepository import with inline implementation
- Created InlineDeploymentRepository class implementing IDeploymentRepository
- Breaks circular dependency: deployment.ts -> prisma/index.ts -> usage-tracking.ts -> deployment.ts
- Verified with madge: no circular dependencies found

Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com>
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@vercel
Copy link
Copy Markdown

vercel Bot commented Oct 21, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Oct 21, 2025 0:10am
cal-eu Ignored Ignored Oct 21, 2025 0:10am

💡 Enable Vercel Agent with $100 free credit for automated AI reviews

@volnei volnei marked this pull request as ready for review October 21, 2025 12:12
@volnei volnei requested a review from a team as a code owner October 21, 2025 12:12
@graphite-app graphite-app Bot requested a review from a team October 21, 2025 12:12
@volnei volnei enabled auto-merge (squash) October 21, 2025 12:26
import { Prisma } from "@calcom/prisma/client";
import type { PrismaClient } from "@calcom/prisma/client";

class InlineDeploymentRepository implements IDeploymentRepository {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we choose a better name?

I don't think inline is a product thing ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is just to make clear that this dependency is Inline cause we have other implementation that isn't. This implementation is intended to be used only inside this file to avoid circular deps.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

@volnei volnei merged commit e310931 into main Oct 21, 2025
85 of 90 checks passed
@volnei volnei deleted the devin/1761046816-fix-circular-dependency-prisma branch October 21, 2025 13:14
@github-actions
Copy link
Copy Markdown
Contributor

E2E results are ready!

KartikLabhshetwar pushed a commit to KartikLabhshetwar/cal.com that referenced this pull request Oct 25, 2025
- Replaced DeploymentRepository import with inline implementation
- Created InlineDeploymentRepository class implementing IDeploymentRepository
- Breaks circular dependency: deployment.ts -> prisma/index.ts -> usage-tracking.ts -> deployment.ts
- Verified with madge: no circular dependencies found

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
akarsh-jain-790 pushed a commit to akarsh-jain-790/cal.com that referenced this pull request Mar 8, 2026
- Replaced DeploymentRepository import with inline implementation
- Created InlineDeploymentRepository class implementing IDeploymentRepository
- Breaks circular dependency: deployment.ts -> prisma/index.ts -> usage-tracking.ts -> deployment.ts
- Verified with madge: no circular dependencies found

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants