ENG-1832 Add Obsidian eslint to the project#1188
Conversation
…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>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR size/scope checkThis PR is over our review-size guideline.
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:
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
| "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" |
There was a problem hiding this comment.
🔴 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.
| "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" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| @@ -1,7 +1,26 @@ | |||
| import { config } from "@repo/eslint-config/react-internal"; | |||
There was a problem hiding this comment.
🚩 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.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
eslint-plugin-obsidianmdto the Obsidian app ESLint config so plugin store review rules surface in the IDEcatalog:obsidianpnpm catalog, leaving roam/website on ESLint 8. ESLint 9 is required forobsidianmd-eslintapps/obsidianfiles in lint-staged and document the workflow inapps/obsidian/AGENTS.mdTest plan
pnpm --dir apps/obsidian lintruns successfully with obsidianmd rules activeMade with Cursor