Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

finding: 两处 eslint-disable-next-line 写错位置,从未生效 —— 作者想压的 warning 至今照常在报 #4850

Closed

Description

@yinlianghui

发现于 #4833 的清理过程(PR #4849),实测基线 origin/main @ 815cad03d。observation-class:
今天没有用户会因此出错,两条都只是 warning,而 .github/workflows/lint.yml 有意不设 --max-warnings。
单独留档是因为它需要一个判断,不是机械修法。

现象

#4833 清掉的 49 条失效指令里,有 2 条的失效原因与其余 47 条不同类。其余 47 条是"被压的问题已经不存在了";
这 2 条是指令写错了位置,从落库起就没生效过 —— eslint-disable-next-line 的"下一行"落在了另一条注释或空行上,
而作者想压的那条 warning 至今仍在照常报。

站点(清理前的行号) 指令 错位方式 作者想压的 warning 现在在哪
apps/console/src/pages/developer/PublicFormsPage.tsx:151 react-hooks/exhaustive-deps 指令是行内块注释,写在 useEffect(() = 那条语句的内部行尾,于是它的"下一行"是 152 行的空行 react-hooks/exhaustive-deps 在 151 行本行照常报着
packages/react/src/SchemaRenderer.tsx:238 @typescript-eslint/no-explicit-any 指令的 -- 理由换行续写到了 239 行,于是"下一行"是那条续写注释 @typescript-eslint/no-explicit-any 在 240 行(type ForwardedProps = Record 那行)照常报着

复现:仓根 pnpm exec eslint . -f json,这两个文件里同时存在「ruleId: null 的失效指令」与「同一条规则的真实 finding」,
行号相邻但不重合 —— 这正是错位的指纹。

为什么 PR #4849 只删不修

#4849 的口径是"清理 ESLint 报为失效的指令",这 2 条按该口径确实是失效的,删除没有争议,已随该 PR 删掉。
但没有把它们挪到正确位置,因为那不是同一件事:

挪位置会让两条现存的、正在报的 warning 被新压制下去 —— 那是抑制语义的变更,而不是清理。
"这两处该不该被抑制"是一个独立判断,#4833 那张零决策的机械卡不该顺带替维护者做掉。

需要判断的是什么

两处各有两条路,选哪条取决于这两处代码本身是否成立,不是格式问题:

A. PublicFormsPage.tsx 的 useEffect(() = … , [])

  • A1:补依赖 / 把 load 包进 useCallback —— 如果空依赖数组其实是 bug(load 变化时不重跑),这才是真修。
  • A2:把指令挪到语句上一行,让它真正生效 —— 如果"只在挂载时跑一次"是刻意的,那就把这个意图落实,并把理由写清楚。

B. SchemaRenderer.tsx 的 type ForwardedProps = Record(开放转发面)

  • B1:把指令改成不换行(理由写进上方已有的 doc 注释,那里其实已经把决定写得很清楚了),让它生效。
  • B2:维持现状不压制 —— 这个 any 有意为之且上方 doc 注释已详述,让它继续计入 no-explicit-any 的总数
    (该规则全仓 warning 里占 79.2%,本来就是已知历史债,多一条少一条无实质差别)。

我的倾向是 B1 + A 需要读代码才能定,但这属于维护者的判断,不预判。

顺带记一笔:同类但无遗留的第三处

packages/app-shell/src/views/RecordDetailView.tsx:1968 也是同样的错位(指令下面跟着 5 行说明性注释),
但它的依赖数组本来就是全的、没有 warning 可压,所以删掉它没有任何遗留,不在本条范围内。

一个可选的机械拦网

这一类"指令写了但没生效"靠人眼看不出来 —— 它和一条正常生效的指令在源码里长得一模一样,
唯一的信号就是 ESLint 那条 Unused eslint-disable directive,而它淹没在 9779 条 warning 里。
#4833 正文提到的 linterOptions.reportUnusedDisableDirectives: 'error' 正好能把这类一次性变红并从此拦住;
#4833 清理完成后该项计数恰好为 0,关门的前置条件已经具备。是否关门是 #4833 遗留的独立决策,一并交分诊。

关联:#4833(清理本体)、PR #4849(删除这 2 条的 PR)、#4806、#4029。

未认领,仅留档。

Activity

  1. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator

    Triage — first-touch grading (triage seat, session session_01KTguWpd7WNGYTGmSNY31RL): promoted to pm:queue, type Task. This does not need the maintainer: both sites are code-level judgments with no contract or product semantics behind them, and no warning gate exists to weaken (lint.yml deliberately sets no --max-warnings).

    Seat guidance for dispatch:

    • B (SchemaRenderer no-explicit-any): take B1 — make the directive effective on one line, pointing at the existing doc comment that already records the deliberate open-forwarding decision. A documented-intentional any that still counts as a warning is noise; the intent is already ruled in-file.
    • A (PublicFormsPage exhaustive-deps): the dev decides from the code, not from this card — if load capturing stale deps is a real re-run bug, fix it (A1); if mount-once is deliberate, place the directive correctly with the reason written (A2). Report which branch was taken and why in the PR body.
    • Out of scope: the reportUnusedDisableDirectives: 'error' gate-closing question belongs to finding: 失效 eslint-disable 已清零(#4833/PR4849),linterOptions.reportUnusedDisableDirectives: 'error' 一次关门的前置已备 —— 门禁强度变更待裁 #4853 — do not fold it in here.

    Generated by Claude Code

  2. added
    domain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repo
    on Aug 21, 2026
  3. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Contributor

    Claim: PM loop round 7
    Session: session_019b5UBNMtTzKbVtZZGvFuxe
    Branch: claude/issue-4850-misplaced-eslint-disable
    Worktree: objectui-issue-4850
    Domain: domain:devx (@ objectui, seat #5748)
    File surface: the two source files carrying the misplaced directives (name them in your report once measured)
    Container & model: S mechanical, mode:subagent, model: sonnet
    Clause-②: no
    Serial constraints cleared: in-flight 0 at round open. Co-dispatched: #5174 batch 3 (content/docs/** + doc-snippet ledger) and #4926 (tsconfig.scripts.json) — both disjoint from source files. ⚠️ #4853 is NOT dispatched and must not be touched — see the coupling note below.

    The card in one line

    Two eslint-disable-next-line directives are on the wrong line, so they have never suppressed anything — the warnings their authors meant to silence are still being reported.

    PM reading — and what I could NOT confirm

    origin/main @ 2aff580b5 carries 229 eslint-disable-next-line occurrences across packages/, apps/ and scripts/ (that count is the positive control that the grep works). ⛔ I did not identify which two are the misplaced ones — locating them is your first task, and the card body is a clue, not a spec.

    Measure, do not eyeball. The honest way to find an ineffective directive is to ask ESLint, not to read code: reportUnusedDisableDirectives reports a directive that suppresses nothing, and running the repo's own lint with that surfaced will name them. If the card's two turn out to be a different set — more, fewer, or different files — report the real set and fix that; the card's count is not binding.

    ⚠️ Coupling with #4853 — read this before touching anything

    #4853 is in the queue, ⛔ not dispatched, and it depends on the state you are about to change. It proposes turning on linterOptions.reportUnusedDisableDirectives: 'error', on the stated precondition that ineffective eslint-disable directives are already at zero (via #4833/PR #4849).

    ⚠️ That precondition and this card cannot both be true. If two directives are misplaced and ineffective, the count is not zero. One of the two cards rests on a stale premise.

    Your measurement settles it, and that is a genuinely valuable output of this run — possibly more valuable than the two-line fix:

    ⛔ Do not enable reportUnusedDisableDirectives: 'error' yourself. That is #4853's decision and a gate-strength change — maintainer-floor. Report your reading and stop at the boundary.

    Direction

    For each genuinely misplaced directive: move it so it actually covers the line its author intended, or delete it if the warning it was aimed at no longer exists. Both are correct outcomes; which applies is per-directive and measured, ⛔ not a blanket choice.

    ⚠️ Watch for the trap this card is made of: a directive that silently suppresses nothing and one that suppresses the wrong thing look identical in a diff. After moving one, confirm the intended warning is actually now suppressed and that no new warning appeared — the whole point is that these were never doing their job, so "lint still passes" ⛔ does not by itself prove you fixed it.

    ⛔ Scope: only the misplaced directives you measured. Other lint debt is a finding to file.

    Gates

    No dispatch-gates.mjs here — derive from your actual diff. At minimum the full root lint task the Lint workflow runs, plus whichever package lint tasks cover the files you touch. Report the warning count before and after — for this card that delta is the evidence. Judge changeset by running check-changeset-presence.mjs; ⛔ no skip-changeset label (never a real mechanism here, per #4912/#3724).


    Generated by Claude Code

  4. added a commit that references this issue on Aug 24, 2026
    e31c811
  5. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Contributor
    {
      "issue": 4850,
      "status": "done",
      "branch": "claude/issue-4850-misplaced-eslint-disable",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5978",
      "premise_still_valid": true,
      "summary": "Measured via `eslint --report-unused-disable-directives`: the two sites named in the card matched exactly (PublicFormsPage.tsx:182 react-hooks/exhaustive-deps, SchemaRenderer.tsx:414 no-explicit-any), no other misplaced directives found repo-wide. Decision A2 for PublicFormsPage (mount-once is deliberate -- refresh is explicit via a Refresh button and post-mutation load() calls, confirmed in code) and B1 for SchemaRenderer (existing doc comment already documents the any as deliberate). Comment-only diff, no logic changes. objectstack-ai/objectui#4853 coupling resolved: on current main the two directives were DELETED by PR objectstack-ai/objectui#4849 (not left present-but-broken), so the strict 'ineffective directive count' is genuinely 0 both before and after this PR -- objectstack-ai/objectui#4853's precondition literally holds. But objectstack-ai/objectui#4849 explicitly deferred a second question (should these two warnings be suppressed at all), which this PR answers; the card's underlying problem (2 warnings the original authors meant to suppress, still firing unsuppressed) was real and is now fixed.",
      "tests": "Targeted: `eslint <file> --report-unused-disable-directives -f json` on both files before/after -- both warnings now suppressed, 0 unused-disable-directive messages in either file (no unrelated warning newly suppressed). Package-level reverse verification (reverted the 2 files to origin/main@2aff580b5, ran lint, restored via git checkout of the committed fix): @object-ui/react 354->353 warnings, @object-ui/console 202->201 warnings. Full root `pnpm lint` (the Lint workflow's gate) after the fix at e31c8116b: 47/47 tasks successful, 0 errors, 10721 warnings total, matching an independent repo-root `eslint . --report-unused-disable-directives -f json` scan (3621 files, 10721 warnings). `node scripts/check-changeset-presence.mjs` passes (empty-frontmatter changeset declared, comment-only change).",
      "open_questions": [],
      "out_of_scope_findings": ["filed as objectstack-ai/objectui#5973: 2 eslint-disable-next-line react-hooks/static-components directives (record-picker i18n tests) now report as unused via --report-unused-disable-directives -- correctly placed, but the rule no longer fires on that pattern (stale, same class as the 47 PR objectstack-ai/objectui#4849 already cleaned, not the misplaced-directive class this card fixed)"]
    }

    Generated by Claude Code

  6. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Contributor

    os-dev-report

    (reposted: the previous comment's <!-- os-dev-report --> HTML-comment marker was stripped by the sanitizer on save, and it also ate a literal <file> placeholder out of the tests field — this repost avoids angle brackets and uses the marker as literal text per the fallback instruction.)

    {
      "issue": 4850,
      "status": "done",
      "branch": "claude/issue-4850-misplaced-eslint-disable",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5978",
      "premise_still_valid": true,
      "summary": "Measured via `eslint --report-unused-disable-directives`: the two sites named in the card matched exactly (PublicFormsPage.tsx:182 react-hooks/exhaustive-deps, SchemaRenderer.tsx:414 no-explicit-any), no other misplaced directives found repo-wide. Decision A2 for PublicFormsPage (mount-once is deliberate -- refresh is explicit via a Refresh button and post-mutation load() calls, confirmed in code) and B1 for SchemaRenderer (existing doc comment already documents the any as deliberate). Comment-only diff, no logic changes. objectstack-ai/objectui#4853 coupling resolved: on current main the two directives were DELETED by PR objectstack-ai/objectui#4849 (not left present-but-broken), so the strict 'ineffective directive count' is genuinely 0 both before and after this PR -- objectstack-ai/objectui#4853's precondition literally holds. But objectstack-ai/objectui#4849 explicitly deferred a second question (should these two warnings be suppressed at all), which this PR answers; the card's underlying problem (2 warnings the original authors meant to suppress, still firing unsuppressed) was real and is now fixed.",
      "tests": "Targeted: `eslint --report-unused-disable-directives -f json` (per file) on both files before/after -- both warnings now suppressed, 0 unused-disable-directive messages in either file (no unrelated warning newly suppressed). Package-level reverse verification (reverted the 2 files to origin/main@2aff580b5, ran lint, restored via git checkout of the committed fix): @object-ui/react 354->353 warnings, @object-ui/console 202->201 warnings. Full root `pnpm lint` (the Lint workflow's gate) after the fix at e31c8116b: 47/47 tasks successful, 0 errors, 10721 warnings total, matching an independent repo-root `eslint . --report-unused-disable-directives -f json` scan (3621 files, 10721 warnings). `node scripts/check-changeset-presence.mjs` passes (empty-frontmatter changeset declared, comment-only change).",
      "open_questions": [],
      "out_of_scope_findings": ["filed as objectstack-ai/objectui#5973: 2 eslint-disable-next-line react-hooks/static-components directives (record-picker i18n tests) now report as unused via --report-unused-disable-directives -- correctly placed, but the rule no longer fires on that pattern (stale, same class as the 47 PR objectstack-ai/objectui#4849 already cleaned, not the misplaced-directive class this card fixed)"]
    }

    Generated by Claude Code

  7. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Contributor

    ACCEPT — PR #5978. Landing on CI convergence, under the maintainer's auto-merge authorisation.

    Reviewer of record: domain:devx @ objectui seat (#5748), round R7, PM session session_019b5UBNMtTzKbVtZZGvFuxe. Verified against the diff and the branch's own files — ⛔ not against the report.

    ⚠️ The PM's framing was wrong, and the measurement is better than either card

    I dispatched this saying #4850 and #4853 cannot both be true — that if two directives are ineffective, #4853's "ineffective count is zero" precondition must be false. The dev measured a third possibility I did not consider:

    On current main the two directives were DELETED by PR #4849 — ⛔ not left present-but-broken.

    So both cards are true at once. #4853's precondition literally holds (strict ineffective-directive count is 0, before and after this PR), and #4850's underlying problem was real — the two warnings the original authors meant to suppress have simply been firing unsuppressed ever since. My "contradiction" was an artefact of assuming #4849 had left the broken directives in place. ⛔ #4853 is not blocked by this and its premise needs no revision.

    ⚠️ What this PR actually does — read before treating it as a nit

    The diff adds two suppressions rather than moving existing ones. That is adjacent to the gate-weakening line, so I did not wave it through — I verified both justifications against the branch's own files rather than the report:

    site claim independently verified
    PublicFormsPage.tsx mount-once by design; refresh is explicit ✅ onClick={load} Refresh button at :338; await load() after mutations at :232 and :303; and the file-level doc comment at :8 already said "refresh is driven by the explicit Refresh button" — the design predates this PR
    SchemaRenderer.tsx the any is deliberate, documented ✅ the pre-existing doc comment reads "The any is the point: this is a pass-through channel, not a typed prop", and explains the key set is unknowable because the component is resolved at runtime from a plugin-extensible registry

    Verdict: not gate weakening. No threshold lowered, no rule disabled, no check removed — these are the per-site escape hatch used with a written reason, on two sites whose deliberateness was documented in the code before this card existed. #4849 explicitly deferred the question "should these two be suppressed at all"; this PR answers it, with the answer already written in the files.

    ⚠️ For the record, so a future reader does not mistake the direction: this PR reduces reported warnings by exactly 2 (measured per package: @object-ui/react 354 → 353, @object-ui/console 202 → 201), and adds zero. It does not suppress anything the authors did not already intend to suppress.

    Method — the right instrument

    The dev asked ESLint, not its own eyes: eslint --report-unused-disable-directives -f json, per file, before and after. It confirmed both directives are now effective and that no unrelated warning was newly suppressed — the second half being the trap I flagged in the dispatch (a directive that suppresses the wrong thing looks identical in a diff to one that suppresses the right thing). Package-level reverse verification was run by reverting the two files to origin/main@2aff580b5 and restoring from the commit.

    Repo-wide sweep confirms the card's count: no other misplaced directives exist. Full root pnpm lint after the fix: 47/47 tasks, 0 errors, cross-checked against an independent repo-root scan of 3,621 files.

    Changeset

    Empty-frontmatter changeset declared (releases nothing) documenting a comment-only change. Correct for this diff — it touches a published package's src/ (packages/react), so the presence gate is satisfied without triggering a version bump for a change with no runtime effect.

    Filed

    #5973 — 2 react-hooks/static-components directives that are correctly placed but now report as unused because the rule no longer fires on that pattern. ⛔ Correctly identified as a different class (stale, like the 47 PR #4849 already cleared) rather than folded in.

    Landing

    Path fork checked against the actual file list first: apps/console/src/**, packages/react/src/** and .changeset/** are not governed surface. Maintainer's ruling, in-session 2026-08-24, verbatim:

    授权 auto-merge(推荐)


    Generated by Claude Code

  8. added a commit that references this issue on Sep 1, 2026
    835a2ce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

domain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repopm:dispatched

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions