-
Notifications
You must be signed in to change notification settings - Fork 11.6k
fix: change no-restricted-imports to @typescript-eslint/no-restricted-imports for allowTypeImports support #23860
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.
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
fix: change no-restricted-imports to @typescript-eslint/no-restricted-imports for allowTypeImports support #23860
Conversation
…-imports for allowTypeImports support The allowTypeImports option is only supported by @typescript-eslint/no-restricted-imports, not the standard ESLint no-restricted-imports rule. This fixes the configuration error: 'Configuration for rule no-restricted-imports is invalid: allowTypeImports should NOT have additional properties' Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
WalkthroughThe pull request updates .eslintrc.js by changing the app-store override rule key from "no-restricted-imports" to "@typescript-eslint/no-restricted-imports". The existing options (error level and patterns with group/message) are preserved. This shifts the restriction to the TypeScript-specific rule without altering the configured patterns or severity. No other files or configurations are modified. Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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.
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. Comment |
There was a problem hiding this 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 (1)
.eslintrc.js (1)
25-36: Optional: limit this override to TS files only.The rule’s type-focused option isn’t needed for JS/JSX. Consider narrowing
filesto**/*.{ts,tsx}for this block to avoid running a TS rule on JS sources.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.eslintrc.js(1 hunks)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Detect changes
🔇 Additional comments (2)
.eslintrc.js (2)
25-36: Good swap to @typescript-eslint/no-restricted-imports; fixes the schema error.This change is correct and aligns with allowTypeImports support. Severity and patterns preserved.
25-36: Verify plugin availability and intended block surface.
- Ensure @typescript-eslint/eslint-plugin is loaded via the preset; otherwise you’ll see “Definition for rule ‘@typescript-eslint/no-restricted-imports’ was not found.”
- If you also want to block root imports like
@calcom/trpc(not just deep paths), add apathsentry:"@typescript-eslint/no-restricted-imports": [ "error", { + paths: [ + { + name: "@calcom/trpc", + message: "tRPC imports are blocked in packages/app-store. Move UI to apps/web/components/apps or introduce an API boundary.", + allowTypeImports: false + } + ], patterns: [ { group: ["@calcom/trpc/*", "@trpc/*"], message: "tRPC imports are blocked in packages/app-store. Move UI to apps/web/components/apps or introduce an API boundary.", allowTypeImports: false, }, ], }, ],
eunjae-lee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the problem seems to be fixed !
E2E results are ready! |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
What does this PR do?
Fixes ESLint configuration error by changing
no-restricted-importsto@typescript-eslint/no-restricted-importsfor app-store files to support theallowTypeImportsoption.Problem: ESLint was failing with configuration error:
Solution: The
allowTypeImportsoption is only supported by the TypeScript ESLint version of the rule, not the standard ESLint rule.Visual Demo (For contributors especially)
Before (ESLint configuration error):
After (ESLint runs successfully):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
yarn lint:reportfrom the repository rootpackages/app-store/**filesallowTypeImports: falsesettingExpected result: ESLint should run successfully without configuration errors while maintaining the same import restriction behavior.
Checklist
Link to Devin run: https://app.devin.ai/sessions/5c84830b94dc4807866fdcc4ac3f77e6
Requested by: @hariombalhara
Human Review Checklist:
@typescript-eslint/no-restricted-importsenforces the same restrictions as the original ruleallowTypeImports: falseworks correctly (type imports should be blocked)