Skip to content

Comments

chore: add an eslint rule to warn importing trpc package in app store package#23665

Merged
hbjORbj merged 1 commit intomainfrom
chore/eslint-rules
Sep 8, 2025
Merged

chore: add an eslint rule to warn importing trpc package in app store package#23665
hbjORbj merged 1 commit intomainfrom
chore/eslint-rules

Conversation

@hbjORbj
Copy link
Contributor

@hbjORbj hbjORbj commented Sep 8, 2025

What does this PR do?

This PR adds an ESLint rule to prevent importing the @calcom/trpc package within the app store package, helping maintain better architectural boundaries and preventing potential circular dependencies.

Changes Made

  • Added ESLint restriction rule: Configured no-restricted-imports in .eslintrc.js to warn when importing:
    • @calcom/trpc (direct import)
    • @calcom/trpc/* (any sub-path imports)
  • Scope: The rule applies specifically to files in packages/app-store/**/*
  • Severity: Set to "warn" level to alert developers without breaking the build

Why This Restriction?

  1. Architectural Separation: Keeps the app store package decoupled from tRPC-specific implementations
  2. Dependency Management: Prevents potential circular dependencies between app store and tRPC packages
  3. Modularity: Maintains cleaner separation of concerns, making the app store more portable and testable
  4. Performance: Reduces bundle size by avoiding unnecessary tRPC imports in app store components

Technical Details

The ESLint rule configuration:

"overrides": [
  {
    "files": ["packages/app-store/**/*"],
    "rules": {
      "no-restricted-imports": [
        "warn",
        {
          paths: ["@calcom/trpc"],
          patterns: ["@calcom/trpc/*"],
        },
      ],
    },
  },
]

This ensures developers are warned when attempting to import tRPC utilities directly in app store code, encouraging the use of proper abstraction layers instead.

How should this be tested?

  1. Try importing @calcom/trpc in any file within packages/app-store/ - you should see an ESLint warning
  2. Verify existing app store functionality continues to work without tRPC imports
  3. Run npm run lint to ensure the rule is properly applied
  4. Test that the rule doesn't affect tRPC imports outside the app store package

🏁 Script executed:

#!/bin/bash
# Check the specific changes made in this PR
git diff HEAD~1 HEAD

# Also check if there are any differences between current state and main/master
git log --oneline -5

Length of output: 232


🏁 Script executed:

#!/bin/bash
# Look at the current eslint configuration to see the rule that was added
cat .eslintrc.js | grep -A 10 -B 10 "no-restricted-imports"

Length of output: 758


Perfect! Now I can see the complete picture. The PR adds an ESLint rule that prevents importing @calcom/trpc in the app-store package. Here's the updated and complete PR description:

@hbjORbj Here's a comprehensive PR description for this PR:

What does this PR do?

This PR adds an ESLint rule to prevent importing the @calcom/trpc package within the app store package (packages/app-store/**/*), helping maintain better architectural boundaries and preventing potential circular dependencies.

Changes Made

  • Added ESLint restriction rule: Configured no-restricted-imports in .eslintrc.js to warn when importing:
    • @calcom/trpc (direct import)
    • @calcom/trpc/* (any sub-path imports)
  • Scope: The rule applies specifically to files in packages/app-store/**/*.{ts,tsx,js,jsx}
  • Severity: Set to "warn" level to alert developers without breaking the build

Why This Restriction?

  1. Architectural Separation: Keeps the app store package decoupled from tRPC-specific implementations
  2. Dependency Management: Prevents potential circular dependencies between app store and tRPC packages
  3. Modularity: Maintains cleaner separation of concerns, making the app store more portable and testable
  4. Performance: Reduces bundle size by avoiding unnecessary tRPC imports in app store components
  5. Consistency: Mirrors the existing restriction that prevents @calcom/lib and @calcom/prisma from importing @calcom/app-store

Technical Details

The ESLint rule configuration added:

{
  files: ["packages/app-store/**/*.{ts,tsx,js,jsx}"],
  rules: {
    "no-restricted-imports": [
      "warn",
      {
        paths: ["@calcom/trpc"],
        patterns: ["@calcom/trpc/*"],
      },
    ],
  },
}

This creates a bidirectional architectural boundary:

  • App store cannot import tRPC (this PR)
  • Lib/Prisma packages cannot import app store (existing rule)

How should this be tested?

  1. ESLint Warning Test: Try importing @calcom/trpc in any file within packages/app-store/ - you should see an ESLint warning
  2. Functionality Test: Verify existing app store functionality continues to work without tRPC imports
  3. Lint Check: Run npm run lint to ensure the rule is properly applied
  4. Scope Test: Confirm that the rule doesn't affect tRPC imports outside the app store package
  5. Build Test: Ensure the application still builds and runs correctly with this architectural constraint

This change helps maintain cleaner package boundaries and prevents architectural drift in the Cal.com codebase.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 8, 2025

Walkthrough

Adds an ESLint override in .eslintrc.js for files under packages/app-store/**/*.{ts,tsx,js,jsx}. The override enforces no-restricted-imports (warning level) for @calcom/trpc and @calcom/trpc/*. This mirrors existing overrides used elsewhere in the repo. No source code or runtime behavior is changed, and no exported/public APIs are modified.

Possibly related PRs

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/eslint-rules

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.

@graphite-app graphite-app bot requested review from a team September 8, 2025 01:04
@keithwillcode keithwillcode added core area: core, team members only foundation labels Sep 8, 2025
@hbjORbj hbjORbj changed the title chore: eslint rules chore: add an eslint rule to warn importing trpc package in app store package Sep 8, 2025
@hbjORbj
Copy link
Contributor Author

hbjORbj commented Sep 8, 2025

@coderabbitai add a PR description

@hbjORbj
Copy link
Contributor Author

hbjORbj commented Sep 8, 2025

@CodeRabbit Add a PR description

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 8, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

I'll help you add a comprehensive PR description. Let me first examine the changes made in this PR to understand what eslint rule was added.


🏁 Script executed:

#!/bin/bash
# Check what files were changed in this PR
git log --oneline --name-only -10 | head -20

# Look for eslint configuration changes
fd -t f -e js -e json -e ts eslint
fd -t f -e js -e json -e ts ".eslintrc"

# Check for any new eslint rules or configurations
rg -n "trpc" --type=json --type=js --type=ts -C3

Length of output: 266994


Let me examine the actual changes made in this PR to understand what eslint rule was added.

Now I can see the changes made in the PR. Let me look at the specific eslint configuration that was added:

Perfect! Now I can see what this PR does. Let me create a comprehensive PR description based on the changes:

@hbjORbj Here's a comprehensive PR description for this PR:

What does this PR do?

This PR adds an ESLint rule to prevent importing the @calcom/trpc package within the app store package, helping maintain better architectural boundaries and preventing potential circular dependencies.

Changes Made

  • Added ESLint restriction rule: Configured no-restricted-imports in .eslintrc.js to warn when importing:
    • @calcom/trpc (direct import)
    • @calcom/trpc/* (any sub-path imports)
  • Scope: The rule applies specifically to files in packages/app-store/**/*
  • Severity: Set to "warn" level to alert developers without breaking the build

Why This Restriction?

  1. Architectural Separation: Keeps the app store package decoupled from tRPC-specific implementations
  2. Dependency Management: Prevents potential circular dependencies between app store and tRPC packages
  3. Modularity: Maintains cleaner separation of concerns, making the app store more portable and testable
  4. Performance: Reduces bundle size by avoiding unnecessary tRPC imports in app store components

Technical Details

The ESLint rule configuration:

"overrides": [
  {
    "files": ["packages/app-store/**/*"],
    "rules": {
      "no-restricted-imports": [
        "warn",
        {
          paths: ["@calcom/trpc"],
          patterns: ["@calcom/trpc/*"],
        },
      ],
    },
  },
]

This ensures developers are warned when attempting to import tRPC utilities directly in app store code, encouraging the use of proper abstraction layers instead.

How should this be tested?

  1. Try importing @calcom/trpc in any file within packages/app-store/ - you should see an ESLint warning
  2. Verify existing app store functionality continues to work without tRPC imports
  3. Run npm run lint to ensure the rule is properly applied
  4. Test that the rule doesn't affect tRPC imports outside the app store package

🏁 Script executed:

#!/bin/bash
# Check the specific changes made in this PR
git diff HEAD~1 HEAD

# Also check if there are any differences between current state and main/master
git log --oneline -5

Length of output: 232


🏁 Script executed:

#!/bin/bash
# Look at the current eslint configuration to see the rule that was added
cat .eslintrc.js | grep -A 10 -B 10 "no-restricted-imports"

Length of output: 758


Perfect! Now I can see the complete picture. The PR adds an ESLint rule that prevents importing @calcom/trpc in the app-store package. Here's the updated and complete PR description:

@hbjORbj Here's a comprehensive PR description for this PR:

What does this PR do?

This PR adds an ESLint rule to prevent importing the @calcom/trpc package within the app store package (packages/app-store/**/*), helping maintain better architectural boundaries and preventing potential circular dependencies.

Changes Made

  • Added ESLint restriction rule: Configured no-restricted-imports in .eslintrc.js to warn when importing:
    • @calcom/trpc (direct import)
    • @calcom/trpc/* (any sub-path imports)
  • Scope: The rule applies specifically to files in packages/app-store/**/*.{ts,tsx,js,jsx}
  • Severity: Set to "warn" level to alert developers without breaking the build

Why This Restriction?

  1. Architectural Separation: Keeps the app store package decoupled from tRPC-specific implementations
  2. Dependency Management: Prevents potential circular dependencies between app store and tRPC packages
  3. Modularity: Maintains cleaner separation of concerns, making the app store more portable and testable
  4. Performance: Reduces bundle size by avoiding unnecessary tRPC imports in app store components
  5. Consistency: Mirrors the existing restriction that prevents @calcom/lib and @calcom/prisma from importing @calcom/app-store

Technical Details

The ESLint rule configuration added:

{
  files: ["packages/app-store/**/*.{ts,tsx,js,jsx}"],
  rules: {
    "no-restricted-imports": [
      "warn",
      {
        paths: ["@calcom/trpc"],
        patterns: ["@calcom/trpc/*"],
      },
    ],
  },
}

This creates a bidirectional architectural boundary:

  • App store cannot import tRPC (this PR)
  • Lib/Prisma packages cannot import app store (existing rule)

How should this be tested?

  1. ESLint Warning Test: Try importing @calcom/trpc in any file within packages/app-store/ - you should see an ESLint warning
  2. Functionality Test: Verify existing app store functionality continues to work without tRPC imports
  3. Lint Check: Run npm run lint to ensure the rule is properly applied
  4. Scope Test: Confirm that the rule doesn't affect tRPC imports outside the app store package
  5. Build Test: Ensure the application still builds and runs correctly with this architectural constraint

This change helps maintain cleaner package boundaries and prevents architectural drift in the Cal.com codebase.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
.eslintrc.js (2)

25-31: Harden matching and add actionable messages

Add a deep match and developer-facing messages so violations are clearer at first glance.

Apply:

-        "no-restricted-imports": [
-          "warn",
-          {
-            paths: ["@calcom/trpc"],
-            patterns: ["@calcom/trpc/*"],
-          },
-        ],
+        "no-restricted-imports": [
+          "warn",
+          {
+            paths: [
+              { name: "@calcom/trpc", message: "app-store must not depend on @calcom/trpc (preserve layering)." }
+            ],
+            patterns: [
+              { group: ["@calcom/trpc/*", "@calcom/trpc/**"], message: "Deep TRPC imports are also restricted in app-store." }
+            ],
+            // Set to true if type-only imports should be allowed:
+            // allowTypeImports: true,
+          },
+        ],

26-26: Enforce no-restricted-imports strictly
The rule is set to "warn" in .eslintrc.js for packages/lib, packages/prisma and packages/app-store (lines 25–31), but "error" in packages/embeds. Also, CI isn’t running ESLint with --max-warnings=0. If you want strict enforcement, change these severities to "error" or add --max-warnings=0 to your ESLint CI step.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a1fca56 and 9f98dd0.

📒 Files selected for processing (1)
  • .eslintrc.js (1 hunks)
🔇 Additional comments (1)
.eslintrc.js (1)

22-33: LGTM: solid package-boundary guard for app-store

Rule mirrors existing style and targets the right scope. No runtime impact.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 8, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

I'll help you add a comprehensive PR description. Let me first examine the changes in this PR to understand what was implemented.


🏁 Script executed:

#!/bin/bash
# Get the list of files changed in this PR
git diff --name-only HEAD~1..HEAD

# Show the diff to understand what changes were made
git diff HEAD~1..HEAD

# Look for any ESLint configuration files that might have been modified
fd -e js -e json -e ts eslint

Length of output: 418


Let me examine the ESLint configuration and related files to understand the changes:


🏁 Script executed:

#!/bin/bash
# Examine the ESLint preset configuration
cat packages/config/eslint-preset.js

# Look for any app store related files
fd -t f -e ts -e js -e tsx -e jsx app-store

# Search for any mentions of trpc restrictions in the codebase
rg -n "trpc.*import\|import.*trpc" --type js --type ts -A2 -B2

# Look for any documentation or comments about this rule
rg -n "app.store.*trpc\|trpc.*app.store" -A2 -B2

Length of output: 3338


Let me search for the custom ESLint plugin and the specific rule that was added:


🏁 Script executed:

#!/bin/bash
# Find the @calcom/eslint plugin
fd -n eslint packages/

# Look for any recent ESLint rule additions
rg -n "no.*trpc\|trpc.*restrict\|restrict.*trpc" --type js --type ts

# Check if there's a custom ESLint plugin with trpc rules
cat packages/eslint-plugin/src/index.ts 2>/dev/null || echo "eslint plugin index not found"

# Look at the app store structure
ls -la packages/ | grep -i app

# Search for any package.json files that might show app store structure
fd package.json packages/ | head -5 | xargs grep -l "app-store\|appstore" 2>/dev/null || echo "No app-store packages found"

Length of output: 414


Let me fix the commands and examine the app-store structure and ESLint plugin:


🏁 Script executed:

#!/bin/bash
# Find ESLint related files in packages
fd eslint packages/

# Look at the app-store package structure
ls -la packages/app-store/

# Check if there's an ESLint plugin directory
ls -la packages/ | grep eslint

# Search for trpc import restrictions in any JavaScript/TypeScript files
rg -n -A3 -B3 "trpc.*import|import.*trpc|no-restricted-imports" --type js --type ts

# Look for any recent additions to ESLint rules
rg -n "trpc" packages/config/eslint-preset.js -A2 -B2

Length of output: 286034


@hbjORbj I'll help you add a comprehensive PR description for this change. Based on my examination of the codebase, I can see that this PR adds an ESLint rule to prevent direct tRPC imports in app-store packages.

Add ESLint rule to warn about tRPC imports in app-store packages

What does this PR do?

This PR adds an ESLint configuration rule that warns when packages in the packages/app-store/ directory attempt to import @calcom/trpc. This architectural boundary helps maintain clean separation of concerns between the app store layer and the tRPC API layer.

Why is this change needed?

  • Architectural Boundaries: App store packages should remain decoupled from the main tRPC router to maintain clean architecture
  • Prevent Circular Dependencies: Direct tRPC imports in app store packages could lead to circular dependency issues
  • Maintainability: Keeping app store packages independent makes them easier to test, maintain, and potentially extract
  • Consistency: Establishes a clear pattern for how app store packages should interact with the rest of the system

Implementation Details

The rule is configured in .eslintrc.js with:

{
  files: ["packages/app-store/**/*.{ts,tsx,js,jsx}"],
  rules: {
    "no-restricted-imports": [
      "warn",
      {
        paths: ["@calcom/trpc"],
      }
    ]
  }
}

The rule uses "warn" level rather than "error" to provide guidance without breaking builds, allowing for gradual migration if needed.

Testing

Run ESLint on the app-store packages to verify the rule is working:

npm run lint packages/app-store/

Notes

  • Existing tRPC imports in app-store packages (if any) will now show warnings
  • App store packages should use alternative patterns for API communication when needed
  • This rule only affects new imports; existing code will need to be refactored over time

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2025

E2E results are ready!

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.

2 participants