-
Notifications
You must be signed in to change notification settings - Fork 6
ENG-1832 Add Obsidian eslint to the project #1188
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -34,7 +34,8 @@ | |||||
| "tailwindConfig": "./packages/tailwind-config/tailwind.config.ts" | ||||||
| }, | ||||||
| "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" | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Impact: Obsidian plugin store guideline violations slip through commits undetected, contrary to what the documentation promises. Mechanism: eslint-plugin-only-warn + missing --max-warnings 0The shared ESLint base config at 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 ( As a result, obsidianmd rule violations produce warnings that ESLint silently accepts, and the lint-staged pre-commit hook succeeds. The claim in To fix, add
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||
| }, | ||||||
| "dependencies": { | ||||||
| "posthog-js": "catalog:" | ||||||
|
|
||||||
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.
🚩 ESLint major version mismatch between shared config package and obsidian app
The shared
@repo/eslint-configpackage declareseslint: catalog:which resolves to ESLint 8.57.1 (pnpm-workspace.yaml:30), whileapps/obsidiannow useseslint: catalog:obsidianresolving to ESLint ^9.30.0 (pnpm-workspace.yaml:47). Similarly,typescript-eslintis 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.