Skip to content

Added ESLint rule to enforce path aliases over relative imports#25355

Merged
rob-ghost merged 1 commit intomainfrom
feat/eslint-relative-imports
Nov 6, 2025
Merged

Added ESLint rule to enforce path aliases over relative imports#25355
rob-ghost merged 1 commit intomainfrom
feat/eslint-relative-imports

Conversation

@rob-ghost
Copy link
Contributor

@rob-ghost rob-ghost commented Nov 5, 2025

ref https://linear.app/ghost/issue/BER-2610

Why are we making this change?

As part of the Ember → React migration, we're adding new React code to apps/admin. We've configured TypeScript path aliases (@/* for src, @test-utils/* for test-utils) to improve code readability and maintainability.

However, developers can still write relative imports (../../utils/foo) even when path aliases are available. This PR enforces the use of path aliases through ESLint to ensure consistency across the codebase.

What does this PR do?

  1. Adds eslint-plugin-no-relative-import-paths to apps/admin/package.json

  2. Configures ESLint rules in apps/admin/eslint.config.js:

    • For src/**/*.{ts,tsx}: Enforces @/* aliases, allows same-folder relative imports
    • For test-utils/**/*.{ts,tsx}: Enforces use of aliases, allows same-folder relative imports
  3. Updates yarn.lock with the new dependency

Why is this needed?

  • Consistency: Ensures all developers use path aliases consistently
  • Refactoring safety: Moving files doesn't break imports when using absolute aliases
  • Readability: @/utils/foo is clearer than ../../../utils/foo
  • Auto-fixable: The ESLint rule can automatically fix violations

Example

// ❌ Before (relative imports)
import { foo } from '../../utils/foo';
import { mockUser } from '../../../test-utils/factories/user';

// ✅ After (path aliases)
import { foo } from '@/utils/foo';
import { mockUser } from '@test-utils/factories/user';

Note

Enforces path alias imports in apps/admin by adding and configuring eslint-plugin-no-relative-import-paths.

  • ESLint (apps/admin):
    • Plugin: Add eslint-plugin-no-relative-import-paths and register it in eslint.config.js.
    • Rules:
      • For src/**/*.{ts,tsx}: Enforce alias imports with { rootDir: 'src', prefix: '@', allowSameFolder: true }.
      • For test-utils/**/*.{ts,tsx}: Enforce no relative imports (allow same-folder).
  • Dependencies:
    • Add eslint-plugin-no-relative-import-paths to devDependencies and update yarn.lock.

Written by Cursor Bugbot for commit 0879916. This will update automatically on new commits. Configure here.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds the ESLint plugin eslint-plugin-no-relative-import-paths to apps/admin/package.json devDependencies and registers it in apps/admin/eslint.config.js. The ESLint config exposes the plugin and adds two overrides: one for src/**/*.{ts,tsx} applying no-relative-import-paths/no-relative-import-paths with options { allowSameFolder: true, rootDir: 'src', prefix: '@' }; and one for test-utils/**/*.{ts,tsx} applying the same rule with { allowSameFolder: true }.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Review changes in apps/admin/eslint.config.js (plugin import, plugins registration, two override rule blocks and option values).
  • Verify apps/admin/package.json devDependency entry for eslint-plugin-no-relative-import-paths and version string.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and clearly summarizes the main change: adding an ESLint rule to enforce path aliases over relative imports, which is the primary focus of this PR.
Description check ✅ Passed The description is well-detailed and directly related to the changeset, providing context, motivation, examples, and clear explanation of what was changed and why.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/eslint-relative-imports

📜 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 54b7d1a and 0879916.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (2)
  • apps/admin/eslint.config.js (3 hunks)
  • apps/admin/package.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/admin/eslint.config.js
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-10-15T07:53:49.814Z
Learning: Applies to apps/shade/**/*.{ts,tsx,js} : Use the @ alias for internal imports (e.g., @/lib/utils)
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-10-15T07:53:49.814Z
Learning: Applies to apps/shade/**/* : Follow ESLint and tailwindcss/* rules
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Setup
  • GitHub Check: Build & Push
  • GitHub Check: Setup
🔇 Additional comments (1)
apps/admin/package.json (1)

31-31: No issues found — package addition is verified and appropriate.

Version 1.6.1 is compatible with ESLint 9 and has no known CVEs or direct vulnerabilities. The package placement, versioning constraint, and maintenance status are all sound.


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.

@rob-ghost rob-ghost added the test temporary label to try to force ci to run label Nov 5, 2025
@rob-ghost rob-ghost self-assigned this Nov 5, 2025
@codecov
Copy link

codecov bot commented Nov 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.15%. Comparing base (3bb2224) to head (0879916).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25355      +/-   ##
==========================================
+ Coverage   72.10%   72.15%   +0.04%     
==========================================
  Files        1515     1515              
  Lines      115104   115109       +5     
  Branches    13781    13790       +9     
==========================================
+ Hits        82995    83053      +58     
+ Misses      31096    31042      -54     
- Partials     1013     1014       +1     
Flag Coverage Δ
admin-tests 51.55% <ø> (+0.01%) ⬆️
e2e-tests 72.15% <ø> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rob-ghost rob-ghost marked this pull request as ready for review November 5, 2025 13:24
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

rules: {
'no-relative-import-paths/no-relative-import-paths': [
'error',
{ allowSameFolder: true, rootDir: 'src', prefix: '@' },
Copy link

Choose a reason for hiding this comment

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

Bug: Mismatched Path Aliases Break ESLint/TS Imports

The ESLint configuration enforces path aliases with prefix: '@' and rootDir: 'src', but there are no corresponding TypeScript path aliases configured in tsconfig.app.json or tsconfig.json. This will cause ESLint to enforce and auto-fix imports to use @/* aliases that are not recognized by TypeScript or the build system, resulting in broken imports and compilation errors. The TypeScript configuration needs to include a paths section mapping @/* to ./src/* (similar to apps/shade, apps/activitypub, and apps/admin-x-settings).

Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These aliases come from #25354

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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7609889 and 54b7d1a.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (2)
  • apps/admin/eslint.config.js (3 hunks)
  • apps/admin/package.json (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-10-15T07:53:49.814Z
Learning: Applies to apps/shade/**/*.{ts,tsx,js} : Use the @ alias for internal imports (e.g., @/lib/utils)
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-09T17:25:12.439Z
Learning: Applies to apps/@(admin-x-*,posts)/** : Admin UI new features must be implemented in React under apps/admin-x-* or apps/posts
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-09T17:25:12.439Z
Learning: Applies to apps/@(admin-x-*,posts)/**/*.{ts,tsx,js,jsx} : Avoid admin-x-design-system for new Admin UI work (legacy, being phased out)
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-10-15T07:53:49.814Z
Learning: Applies to apps/shade/**/* : Follow ESLint and tailwindcss/* rules
📚 Learning: 2025-10-15T07:53:49.814Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-10-15T07:53:49.814Z
Learning: Applies to apps/shade/**/*.{ts,tsx,js} : Use the @ alias for internal imports (e.g., @/lib/utils)

Applied to files:

  • apps/admin/eslint.config.js
📚 Learning: 2025-10-15T07:53:49.814Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-10-15T07:53:49.814Z
Learning: Applies to apps/shade/test/unit/**/*.test.@(ts|tsx|js) : Unit tests live under test/unit with *.test.(ts|tsx|js) naming

Applied to files:

  • apps/admin/eslint.config.js
📚 Learning: 2025-10-15T07:53:49.814Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-10-15T07:53:49.814Z
Learning: Applies to apps/shade/**/* : Follow ESLint and tailwindcss/* rules

Applied to files:

  • apps/admin/eslint.config.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Cursor Bugbot
  • GitHub Check: E2E Tests (Shard 2/2)
  • GitHub Check: E2E Tests (Shard 1/2)
🔇 Additional comments (3)
apps/admin/eslint.config.js (2)

7-7: LGTM! Plugin import and registration.

The plugin import and registration follow the correct ESLint flat config format.

Also applies to: 19-21


31-40: LGTM! Correct configuration for src files.

The rule configuration correctly enforces @/* aliases for src files with auto-fix support. The allowSameFolder: true option provides reasonable flexibility for same-directory imports.

apps/admin/package.json (1)

30-30: No issues found. The dependency version is current and secure.

The version ^1.6.1 is the latest available on npm, and there are no known security vulnerabilities for this package. The dependency addition is correct and safe to merge.

@rob-ghost rob-ghost enabled auto-merge (squash) November 6, 2025 10:58
ref https://linear.app/ghost/issue/BER-2610

- Configured no-relative-import-paths ESLint plugin
- Enforces use of @ aliases instead of relative imports in src/
- Enforces use of @test-utils/* instead of relative imports in test-utils/
- Allows same-folder relative imports for convenience
@rob-ghost rob-ghost force-pushed the feat/eslint-relative-imports branch from 54b7d1a to 0879916 Compare November 6, 2025 11:11
@rob-ghost rob-ghost merged commit 617bb94 into main Nov 6, 2025
31 checks passed
@rob-ghost rob-ghost deleted the feat/eslint-relative-imports branch November 6, 2025 11:22
abdultalha0862 pushed a commit to abdultalha0862/Ghost that referenced this pull request Nov 20, 2025
…host#25355)

ref https://linear.app/ghost/issue/BER-2610

Adds linting rule to not allow relative imports unless its a sibling or sub directory.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test temporary label to try to force ci to run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants