Repository navigation
chore(mosaic): add anti-slop lint rules - #10110
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (30)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
💤 Files with no reviewable changes (6)
Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. 📝 WalkthroughWalkthroughThe Mosaic ESLint configuration adds control-flow, type-assertion, exhaustive-switch, file-length, and Reflect checks. The changes add and test DOM and ref type guards, then use them in components before forwarding refs or accessing DOM APIs. Other changes update CSS property keys, utility imports, and context prop merging. The first-factor and sign-in machines, their tests, and a related architecture reference entry are removed. Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change adds lint rules and replaces type casts with runtime checks. No concrete merge-blocking risk was found. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 22 files. (2 skipped: 2 unsupported.)
Comment |
🦋 Changeset detectedLatest commit: 6276cd3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-biometrics
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/mosaic
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
d5b762d to
d56e2de
Compare
There was a problem hiding this comment.
Worth a discussion if we should have these here or in the files as inline comments? Or maybe we should have these here now for the "todo" after enabling the rule, but any future intentional disables should be inline with a reason? I think I prefer the second and I think there's a rule that enforces adding a reason too which I like.
There was a problem hiding this comment.
yeah, goal is to chip away at this list. preference to get the rules in place and iterate
d56e2de to
c13d350
Compare
c13d350 to
c1280dc
Compare
c1280dc to
cb8be77
Compare
cb8be77 to
6276cd3
Compare
Description
Stacked on #10109. Adds lint rules to
@clerk/mosaicthat catch common AI-generated code patterns, based on dmmulroy/anti-slop:@typescript-eslint/consistent-type-assertions(assertionStyle: 'never'): noas Typecasts.as constis still allowed.@typescript-eslint/switch-exhaustiveness-checksonarjs/no-collapsible-if,no-identical-functions,no-redundant-jump,prefer-single-boolean-returnmax-lines(1000)no-restricted-propertiesbansReflect.get/Reflect.applyOnly the type-assertion rule has existing hits. This PR removes 36 of them:
Record<string, unknown>was unnecessary.refit passes at runtime. A newisRefguard replaces casting it.isElement/isHTMLElementguards inprimitives/utils/dom.ts.Object.keys(...) as (keyof T)[]uses the existingkeysOfhelper, moved toprimitives/utilsso primitives can use it.src/machines/(first-factor-machine,sign-in-machine) is removed. Nothing imported it outside its own tests.The remaining 37 are grandfathered in
eslint-suppressions.json. They are mostly generic internals inmachine/*anduse-render, plus floating-ui interop and refs where removing the cast would change a public prop type.noUncheckedIndexedAccessfollows in #10111.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change