Skip to content

ENG-1832 Add Obsidian eslint to the project#1188

Open
trangdoan982 wants to merge 1 commit into
mainfrom
eng-1832-add-obsidian-eslint-to-the-project
Open

ENG-1832 Add Obsidian eslint to the project#1188
trangdoan982 wants to merge 1 commit into
mainfrom
eng-1832-add-obsidian-eslint-to-the-project

Conversation

@trangdoan982

@trangdoan982 trangdoan982 commented Jul 4, 2026

Copy link
Copy Markdown
Member

Summary

  • Add eslint-plugin-obsidianmd to the Obsidian app ESLint config so plugin store review rules surface in the IDE
  • Scope ESLint 9 and obsidianmd dependencies to the Obsidian app via a catalog:obsidian pnpm catalog, leaving roam/website on ESLint 8. ESLint 9 is required for obsidianmd-eslint
  • Run Obsidian ESLint on changed apps/obsidian files in lint-staged and document the workflow in apps/obsidian/AGENTS.md

Test plan

  • pnpm --dir apps/obsidian lint runs successfully with obsidianmd rules active
  • CI lint-changed-files job passes on this PR
  • Pre-commit lint-staged runs eslint on edited Obsidian TS/TSX files

Made with Cursor


Open in Devin Review

…iance

Layer Obsidian's official ESLint rules onto the obsidian app so store review
risks surface in the IDE and pre-commit, without affecting roam/website.

Co-authored-by: Cursor <cursoragent@cursor.com>
@linear-code

linear-code Bot commented Jul 4, 2026

Copy link
Copy Markdown

ENG-1832

@vercel

vercel Bot commented Jul 4, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
discourse-graph Ready Ready Preview, Comment Jul 4, 2026 6:04am

Request Review

@graphite-app

graphite-app Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

PR size/scope check

This PR is over our review-size guideline.

  • Recommended: ~200 lines changed
  • Acceptable limit: up to 400 lines when well-scoped/self-contained
  • Preferred file count: fewer than 5 files

Please split this into smaller PRs unless there is a clear reason the changes need to land together.

If keeping it as one PR, please add a brief justification covering:

  • What single problem this PR solves
  • Why the files/changes are coupled

@supabase

supabase Bot commented Jul 4, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

Open in Devin Review

Comment thread package.json
"lint-staged": {
"*.{js,mjs,ts,tsx,md,mdx}": "prettier -w"
"*.{js,mjs,ts,tsx,md,mdx}": "prettier -w",
"apps/obsidian/**/*.{ts,tsx}": "pnpm --dir apps/obsidian exec eslint --fix"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Obsidian lint rules silently pass instead of blocking commits as intended

The commit-hook linter is run without a --max-warnings flag (eslint --fix at package.json:38), so rule violations downgraded to warnings by the shared only-warn plugin exit with code 0 instead of failing, and the new AGENTS.md incorrectly states they "still fail lint and block commits."

Impact: Obsidian plugin store guideline violations slip through commits undetected, contrary to what the documentation promises.

Mechanism: eslint-plugin-only-warn + missing --max-warnings 0

The shared ESLint base config at packages/eslint-config/base.js:5-6 loads eslint-plugin-only-warn, which converts every ESLint error into a warning. The obsidianmd rules are layered on top of this shared config (apps/obsidian/eslint.config.mjs:14-23), so they also become warnings.

ESLint's default behavior is to exit with code 0 when only warnings are present (exit code 1 requires at least one error). Neither the lint script ("lint": "eslint ." in apps/obsidian/package.json:10) nor the lint-staged command (pnpm --dir apps/obsidian exec eslint --fix in package.json:38) passes --max-warnings 0.

As a result, obsidianmd rule violations produce warnings that ESLint silently accepts, and the lint-staged pre-commit hook succeeds. The claim in apps/obsidian/AGENTS.md:11 that warnings "still fail lint and block commits via lint-staged" is incorrect.

To fix, add --max-warnings 0 to both the lint script and the lint-staged command, or remove eslint-plugin-only-warn from the obsidian config chain.

Suggested change
"apps/obsidian/**/*.{ts,tsx}": "pnpm --dir apps/obsidian exec eslint --fix"
"apps/obsidian/**/*.{ts,tsx}": "pnpm --dir apps/obsidian exec eslint --max-warnings 0 --fix"
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@@ -1,7 +1,26 @@
import { config } from "@repo/eslint-config/react-internal";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚩 ESLint major version mismatch between shared config package and obsidian app

The shared @repo/eslint-config package declares eslint: catalog: which resolves to ESLint 8.57.1 (pnpm-workspace.yaml:30), while apps/obsidian now uses eslint: catalog:obsidian resolving to ESLint ^9.30.0 (pnpm-workspace.yaml:47). Similarly, typescript-eslint is at ^7.18.0 in the shared package (packages/eslint-config/package.json:26) but ^8.35.1 in obsidian (pnpm-workspace.yaml:51). The obsidian eslint config imports and spreads the shared config (apps/obsidian/eslint.config.mjs:1,15), so the shared config's code (written for ESLint 8 / typescript-eslint 7) runs under ESLint 9 / typescript-eslint 8 at runtime. This works today because ESLint 9 flat config is backward-compatible with the patterns used, but it's fragile — a future change to the shared config could break obsidian's lint, or vice versa. Worth considering whether the shared config should be upgraded to ESLint 9 monorepo-wide or whether obsidian should have a fully independent config.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant