Disable 'import-x/no-rename-default' rule in ESLint config and refactor import #32
Conversation
📝 WalkthroughWalkthroughMultiple patch updates were documented for ESLint-related packages including Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
Scope: all 4 workspace projects This error happened while installing a direct dependency of /tmp/eslint/packages/eslint-presets/eslint-preset-node Packages found in the workspace: ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.changeset/flat-worlds-like.md (1)
5-5: Fix the framework name spelling.Use the official spelling "Next.js" instead of "NextJS" for consistency with the framework's branding.
-Fix ESLint NextJS config +Fix ESLint Next.js config🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: The official spelling of this programming framework is “Next.js”.
Context: ...int-config-next': patch --- Fix ESLint NextJS config(NODE_JS)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/flat-worlds-like.md(1 hunks)eslint.config.mjs(1 hunks)packages/eslint/eslint-config-next/src/next.linter.ts(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/flat-worlds-like.md
[uncategorized] ~5-~5: The official spelling of this programming framework is “Next.js”.
Context: ...int-config-next': patch --- Fix ESLint NextJS config
(NODE_JS)
🔇 Additional comments (1)
packages/eslint/eslint-config-next/src/next.linter.ts (1)
1-1:❓ Verification inconclusive
Verify the import style change is necessary.
The refactor from named import to default import with destructuring changes how
flatConfigis accessed. Ensure this change aligns with the current export structure of@next/eslint-plugin-next.Also applies to: 5-5
🏁 Script executed:
#!/bin/bash # Description: Check the export structure of @next/eslint-plugin-next to verify the import change is necessary # Expected: Confirm whether flatConfig is available as named export or only through default export # Check if package.json references the Next.js ESLint plugin rg -A 5 -B 5 "@next/eslint-plugin-next" package.json # Search for any other usage patterns of this import in the codebase rg -A 3 -B 3 "from '@next/eslint-plugin-next'"Length of output: 459
Verify
flatConfigimport matches the plugin’s current exportsBefore approving, confirm whether
@next/eslint-plugin-nextactually exportsflatConfigas a named export or only via its default export. This will ensure the import in packages/eslint/eslint-config-next/src/next.linter.ts is correct.• Check the published plugin’s export signatures (e.g., inspect its type definitions in node_modules or its repository README).
• IfflatConfigis a named export, revert toimport { flatConfig } from '@next/eslint-plugin-next'.
• If it’s only exposed on the default export, keepimport nextPlugin from '@next/eslint-plugin-next'and destructure.File needing confirmation:
- packages/eslint/eslint-config-next/src/next.linter.ts:1
🦋 Changeset detectedLatest commit: 26edbe5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/eslint/eslint-config/src/base.linter.ts (1)
11-12: Consider more robust project root detection.Using
process.cwd()assumes ESLint runs from the project root, which may not always be the case in monorepos or when ESLint is invoked from subdirectories.Consider using a more robust approach to find the project root:
-const cwdURL = `file://${process.cwd()}/`; -const gitignorePath = fileURLToPath(new URL(`.gitignore`, cwdURL)); +// Find project root by looking for package.json or .git directory +import { findUp } from 'find-up'; +const projectRoot = findUp.sync(['.git', 'package.json'], { type: 'directory' }) || process.cwd(); +const gitignorePath = path.join(projectRoot, '.gitignore');However, this would require adding
find-upas a dependency. The current approach is acceptable for most use cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/empty-banks-stay.md(1 hunks).changeset/plain-buckets-tease.md(1 hunks).changeset/silly-goats-crash.md(1 hunks)packages/eslint-presets/eslint-preset-next/src/next-preset.linter.ts(1 hunks)packages/eslint/eslint-config-react/src/react.linter.ts(1 hunks)packages/eslint/eslint-config/package.json(1 hunks)packages/eslint/eslint-config/src/base.linter.ts(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- .changeset/silly-goats-crash.md
- packages/eslint/eslint-config-react/src/react.linter.ts
- packages/eslint/eslint-config/package.json
- .changeset/plain-buckets-tease.md
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: TheCodeDestroyer
PR: TheCodeDestroyer/devkit#5
File: packages/eslint/eslint-config-react/package.json:80-80
Timestamp: 2025-05-23T18:25:39.851Z
Learning: The @tcd-devkit ESLint configuration packages (like eslint-config-react) are built from scratch specifically for ESLint v9+ using the flat config format, not migrated from older ESLint versions.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: main
🔇 Additional comments (5)
packages/eslint-presets/eslint-preset-next/src/next-preset.linter.ts (1)
14-14: LGTM! Appropriate ignore pattern for Next.js.Adding the
.nextdirectory to the global ignores is correct as it contains generated build files that shouldn't be linted.packages/eslint/eslint-config/src/base.linter.ts (3)
14-21: Clean conditional logic for .gitignore handling.The implementation correctly handles the case where
.gitignoredoesn't exist by using an empty object fallback, preventing configuration errors.
22-24: Proper integration of .gitignore config.The
gitignoreConfigis correctly prepended to the configuration array, ensuring .gitignore patterns are applied before other rules.
1-4:✅ Verification successful
Good implementation approach with minor considerations.
The imports look correct for implementing .gitignore integration. The use of Node.js built-in modules and
@eslint/compatfollows ESLint's recommended patterns for flat config.Verify that
@eslint/compatis properly added as a dependency:
🏁 Script executed:
#!/bin/bash # Description: Check if @eslint/compat is listed as a dependency # Expected: The package should be listed in dependencies or devDependencies fd "package.json" packages/eslint/eslint-config --exec cat {} | jq '.dependencies, .devDependencies'Length of output: 451
Dependency Verification Complete – No Further Action Needed
Verified that
@eslint/compat(v1.2.9) is listed under dependencies in packages/eslint/eslint-config/package.json. All imports and config logic look correct—approving these changes..changeset/empty-banks-stay.md (1)
1-6: Well-formatted changeset documentation.The changeset correctly documents the patch-level update for adding .gitignore support to the ESLint configuration package.
TheCodeDestroyer
force-pushed
the
fix-eslint-config-next
branch
from
4a6024f to
26edbe5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.changeset/flat-worlds-like.md (1)
1-6: Use official "Next.js" branding and clarify the changeset descriptionThe description currently reads:
Fix ESLint NextJS config
- The framework is officially spelled “Next.js”.
- It would be more informative to mention that this patch disables the
import-x/no-rename-defaultrule and refactors theflatConfigimport.Apply this diff:
--- - Fix ESLint NextJS config + Fix ESLint Next.js config by disabling `import-x/no-rename-default` rule and refactoring `flatConfig` import🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: The official spelling of this programming framework is “Next.js”.
Context: ...int-config-next': patch --- Fix ESLint NextJS config(NODE_JS)
packages/eslint/eslint-config-ts/src/__tests__/rules/unbound-method.test.ts (1)
1-18: LGTM with debugging cleanup needed.The test structure and logic are correct. The test properly verifies that the
@typescript-eslint/unbound-methodrule is disabled by checking it equals['off']. The use of theCustomRuletype provides good type safety.However, the console.log statements appear to be debugging code that should be removed before merging.
Apply this diff to remove the debugging statements:
- console.log('tsRules:', tsRules); - console.log('ruleId:', ruleId); - console.log('ruleValue:', ruleValue); -packages/eslint/eslint-config-ts/src/ts.rules.ts (1)
25-26: Remove debugging console.log statement.This console.log statement appears to be debugging code that should be removed before merging to production.
Apply this diff to remove the debugging statement:
-console.log('source tsRules:', tsRules); -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
.changeset/empty-banks-stay.md(1 hunks).changeset/flat-worlds-like.md(1 hunks).changeset/plain-buckets-tease.md(1 hunks).changeset/public-taxis-matter.md(1 hunks).changeset/silly-goats-crash.md(1 hunks)eslint.config.mjs(1 hunks)packages/eslint-presets/eslint-preset-next/src/next-preset.linter.ts(1 hunks)packages/eslint/eslint-config-next/src/next.linter.ts(1 hunks)packages/eslint/eslint-config-react/src/react.linter.ts(1 hunks)packages/eslint/eslint-config-ts/src/__tests__/rules/unbound-method.test.ts(1 hunks)packages/eslint/eslint-config-ts/src/ts.rules.ts(2 hunks)packages/eslint/eslint-config/package.json(1 hunks)packages/eslint/eslint-config/src/base.linter.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/public-taxis-matter.md
🚧 Files skipped from review as they are similar to previous changes (9)
- .changeset/plain-buckets-tease.md
- .changeset/silly-goats-crash.md
- packages/eslint-presets/eslint-preset-next/src/next-preset.linter.ts
- .changeset/empty-banks-stay.md
- packages/eslint/eslint-config/package.json
- packages/eslint/eslint-config-next/src/next.linter.ts
- eslint.config.mjs
- packages/eslint/eslint-config/src/base.linter.ts
- packages/eslint/eslint-config-react/src/react.linter.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: TheCodeDestroyer
PR: TheCodeDestroyer/devkit#32
File: eslint.config.mjs:5-10
Timestamp: 2025-05-31T20:36:11.688Z
Learning: The NextJS ESLint plugin (@next/eslint-plugin-next) exports a default named plugin that requires importing as a default export and then destructuring the needed configuration (e.g., flatConfig). This pattern triggers the `import-x/no-rename-default` rule, necessitating a global disable of this rule when using NextJS ESLint configurations.
Learnt from: TheCodeDestroyer
PR: TheCodeDestroyer/devkit#5
File: packages/eslint/eslint-config-react/package.json:80-80
Timestamp: 2025-05-23T18:25:39.851Z
Learning: The @tcd-devkit ESLint configuration packages (like eslint-config-react) are built from scratch specifically for ESLint v9+ using the flat config format, not migrated from older ESLint versions.
🧬 Code Graph Analysis (1)
packages/eslint/eslint-config-ts/src/__tests__/rules/unbound-method.test.ts (1)
packages/eslint/eslint-config-ts/src/ts.rules.ts (2)
CustomRule(27-27)tsRules(3-23)
🪛 LanguageTool
.changeset/flat-worlds-like.md
[uncategorized] ~5-~5: The official spelling of this programming framework is “Next.js”.
Context: ...int-config-next': patch --- Fix ESLint NextJS config
(NODE_JS)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: main
🔇 Additional comments (1)
packages/eslint/eslint-config-ts/src/ts.rules.ts (1)
12-12: LGTM! Rule addition is correct.The addition of
'@typescript-eslint/unbound-method': ['off']follows the established pattern and correctly disables the rule as intended.
Summary by CodeRabbit
.gitignorefiles to exclude ignored paths from linting..nextdirectory.@typescript-eslint/unbound-methodrule by default in the TypeScript ESLint configuration.