chore(lint): turn on the rules that would have caught the dead code - #7037
Merged
Conversation
Three rules were off, so nothing enforced them. Measured, fixed the sites, and enabled them where the cost is bounded. `noAccumulatingSpread` — 2 violations, both real O(n²) reducers, both now `Object.fromEntries`. One duplicates a block's subBlocks on every block duplication; the other rebuilds a Record from every workspace env var. Enabled repo-wide. `noUnusedVariables` / `noUnusedFunctionParameters` — 633 repo-wide, but only 6 under `packages/`. Fixed those 6 and enabled both at error for `packages/**` via an override, which permanently covers 979 files. `apps/sim`'s remaining 627 are left deliberately: that is a sweep of its own, and a rule enabled with 627 outstanding warnings teaches people to ignore it. This is the class of rule whose absence let the dead code in #7019 accumulate — eleven unread loggers, a whole unimported file, write-only locals — none of which any gate could see. Two of the six were in `workflow-renderer`, where the fix is narrower than it looks. `isWorkflowRunning` is destructured-but-unread in both the block and subflow views, and the app passes it from `workflow-block.tsx` and `subflow-node.tsx`. Its TSDoc claimed it "holds every block's action swell open"; nothing reads it, so that behavior does not exist. Removing the prop breaks the callers and implementing it is a UX decision — there is adjacent logic deliberately not pinning the toolbar during a handoff. So only the unused binding goes, and the TSDoc now says what is true. Not enabled: `noDocumentCookie` (3 sites, and its fix is the CookieStore API, which is a browser-support call) and `useExhaustiveDependencies` (384 errors).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview The two spread-in- Reviewed by Cursor Bugbot for commit 67ad7eb. Configure here. |
Contributor
Greptile SummaryThis PR enables accumulating-spread linting repository-wide and unused-binding linting for
Confidence Score: 5/5The PR appears safe to merge, with the collection rewrites and unused-binding cleanups preserving existing behavior. The new lint scopes are valid, both performance rewrites retain the reducers’ key and value semantics, and all removed bindings were demonstrably unused.
|
| Filename | Overview |
|---|---|
| biome.json | Enables noAccumulatingSpread globally and scopes unused-variable and unused-parameter errors to package files using a valid override. |
| apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx | Replaces an accumulating-spread reducer with an equivalent linear-time environment-variable record construction. |
| apps/sim/stores/workflows/workflow/store.ts | Rebuilds cloned subblocks with Object.fromEntries while preserving per-value structured cloning and key semantics. |
| packages/testing/src/mocks/socket.mock.ts | Marks an intentionally unused mock callback parameter with the underscore convention recognized by the configured lint rule. |
| packages/ts-sdk/src/index.ts | Removes an unused catch binding from workflow validation without changing its false-on-error behavior. |
| packages/workflow-persistence/src/subflow-helpers.ts | Removes unused tuple bindings while preserving loop and parallel conversion behavior. |
| packages/workflow-renderer/src/subflow/subflow-node-view.tsx | Stops destructuring an unread prop and accurately documents its retained compatibility-only status. |
| packages/workflow-renderer/src/workflow-block/workflow-block-view.tsx | Stops destructuring an unread prop and accurately documents that no hold-open behavior is implemented by the view. |
Reviews (1): Last reviewed commit: "chore(lint): turn on the rules that woul..." | Re-trigger Greptile
This was referenced Aug 24, 2026
waleedlatif1
added a commit
that referenced
this pull request
Aug 24, 2026
* refactor(auth): drop the isProduction prop nothing on the signin path reads Same shape as the `isWorkflowRunning` removal: declared, required, threaded through every layer, and never read at the end of the chain. `SocialLoginButtons` declares `isProduction: boolean` as a REQUIRED prop and never reads it, so every caller had to produce and forward a value that was discarded. Neither `login-form` nor `signup-form` reads it either — each only declares it, destructures it, and passes it down. `signup-form` forwards it twice, through its own inner `SignupFormContent` hop. With the chain gone, `getOAuthProviderStatus` has no consumer for the `isProduction: isProd` it returned: the pages destructured it only to forward it, and `/api/auth/providers` already takes just the three availability flags. So the return value and its `isProd` import go too. `isProduction` stays alive where it is genuinely used — `verify-content.tsx` branches on it and hands it to `useVerification`, and imports `isProd` directly rather than through this helper. That path is untouched. Found by the rule enabled in #7037: it was the only `.tsx` unused-parameter warning in `apps/sim`. (cherry picked from commit 331c2ed) * refactor: drop two more props declared, threaded, and never read Same shape as the two already in this PR, found by sweeping the rest of the unused-parameter list for params callers actively compute and pass. `FieldItem.level` is the worse of the two. It is a required `level: number` that the component never reads, and `FieldTreeNodes` exists to thread it: declared, destructured, handed to `FieldItem`, and incremented on every recursion (`level={level + 1}`) from a `level={0}` seed. So a depth counter was carried through an arbitrarily deep tree to feed a component that ignores it. Indentation comes from the nested wrapper divs (`ml-1.5 pl-2.5`, `ml-3 pl-2.5`), not from the counter — removing it changes no rendering. `useMentionMenu`'s `onContextSelect` is a required prop carrying the TSDoc "Callback when a context is selected". The hook never invokes it, so that contract is unimplemented and a future caller would reasonably rely on it. Only the dead hand-off goes there. `addContextNotified` stays: the caller invokes it directly at five sites, and the ref sinks behind it keep its identity stable for those. Context selection has always worked because the caller does the work itself, not because the hook calls back. (cherry picked from commit 110ba76) * refactor: drop two more dead prop chains in the sub-block editor `GroupedCheckboxList` declares `title` (required) and `maxHeight` and reads neither. It renders its own hardcoded copy instead — `Select PII Types to Detect` for the header and `PII types` for the field label — so a block author who sets `title` on a `grouped-checkbox-list` subBlock gets silence, and the `maxHeight = 400` default implies a scroll ceiling that is never applied. Both props go, along with the two values `sub-block.tsx` was passing. `flatTagList` was threaded through the recursive tag renderers to a dead end: declared on `NestedTagRendererProps`, inherited by `FolderContentsProps`, destructured in both, forwarded once more, and read by neither. Its real consumer is `flatTagIndexMap`, built from it at the top level and documented "Map from tag string to index for O(1) lookups" — so the array was being carried alongside its own index through arbitrary nesting depth. The top-level memo and its length checks stay; only the descent goes. Note the component's copy is PII-specific while its name and props present as generic. Renaming it is a separate call, not made here. Both removals were caught mid-flight by `tsc`: my line patterns also matched a live `flatTagList` on `KeyboardNavigationHandler` and a live `title` on `Switch`, which is exactly why the type-check runs before the commit and not after. (cherry picked from commit 5db44f3) * refactor(custom-blocks): drop the workspaceId three mutation hooks never use `usePublishCustomBlock`, `useUpdateCustomBlock` and `useDeleteCustomBlock` each take `workspaceId?: string` and never read it. `custom-block-detail.tsx` passes it to all three. The parameter looks like it was meant to narrow the invalidation to `customBlockKeys.list(workspaceId)`, but `lists()` is the level CLAUDE.md's targeted-invalidation rule actually prescribes, and it is a correct superset. So the invalidation is right as written and the parameter is simply vestigial — removing it is the honest fix, and narrowing the key would be a separate call with its own risk of under-invalidating. Worth recording that these three were reported to me as having zero callers and therefore being dead exports. They are not: the search that produced that claim omitted `apps/sim/ee`, where all three are used. (cherry picked from commit 2d0854a)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three lint rules were
"off", so nothing enforced them. I measured each, fixed the sites, and enabled them where the cost is bounded.What changed
performance/noAccumulatingSpreadcorrectness/noUnusedVariables+noUnusedFunctionParameterspackages/errorforpackages/**suspicious/noDocumentCookieThe accumulating spreads were real
Both are O(n²) reducers, now
Object.fromEntries:stores/workflows/workflow/store.ts— rebuilds a block'ssubBlockson every block duplicationsecrets-manager.tsx— rebuilds aRecordfrom every workspace env varWhy
packages/**and not everywhereapps/simcarries 627 of the 633. That is a sweep of its own, and a rule switched on with 627 outstanding warnings teaches people to scroll past it. Scoped topackages/**, the cost was 6 fixes and the coverage is 979 files, permanently.This is the rule class whose absence let #7019 happen — eleven unread loggers, a whole unimported file, write-only locals. No gate could see any of it.
One fix is narrower than it looks
Two of the six were
isWorkflowRunninginworkflow-renderer: destructured, never read, in both the block and subflow views. My first instinct was to delete the prop — that was wrong twice over, and type-check caught it:apps/simdoesn't)workflow-block.tsx:1243andsubflow-node.tsx:78Its TSDoc claimed it "holds every block's action swell open." Nothing reads it, so that behavior does not exist today. Implementing it is a UX call — there is adjacent logic deliberately not pinning the toolbar during a handoff — and deleting the prop breaks two callers.
So only the unused binding goes, the prop stays in the interface, and the TSDoc now states the truth instead of describing behavior that isn't there. Wiring it up or dropping it from the callers is a follow-up for someone who knows the intended UX.
Not enabled
noDocumentCookie(3 sites, all in the sidebar store) — its fix is theCookieStoreAPI, which is a browser-support decision, not a lint cleanup.useExhaustiveDependencies— 384 errors. Noted, not attempted.Verification
bun run lint:checkexits 0workflow-renderer,workflow-persistence,stores/workflows, and settingslint:checkred, restoring it goes greenOne pre-existing
suppressions/unusedwarning inshell-layout.test.tsis untouched — it is on staging too and unrelated.