Skip to content

fix(staffml): restore working eslint config and fix errors it surfaces - #1981

Merged
profvjreddi merged 1 commit into
harvard-edge:devfrom
Shashank-Tripathi-07:fix/staffml-eslint-10-incompatible-with-eslint-config-next
Aug 10, 2026
Merged

fix(staffml): restore working eslint config and fix errors it surfaces#1981
profvjreddi merged 1 commit into
harvard-edge:devfrom
Shashank-Tripathi-07:fix/staffml-eslint-10-incompatible-with-eslint-config-next

Conversation

@Shashank-Tripathi-07

Copy link
Copy Markdown
Collaborator

Summary

Ran a full audit of StaffML (interviews/staffml): installed deps, ran tsc, eslint, vitest, launched the dev server, and drove it end to end with Playwright across all pages. This PR is the first fix that audit turned up.

Problem

interviews/staffml/package.json pins eslint to 10.4.0 (bumped from 9.39.4 in a prior dependency-bump commit). eslint-config-next@16.2.6 pulls in its own nested eslint-plugin-react@7.37.5, whose peer range only reaches eslint@^9.7 (confirmed via npm view eslint-plugin-react peerDependencies). Running npm run lint crashes outright:

TypeError: Error while loading rule 'react/display-name': contextOrFilename.getFilename is not a function

CI never caught this because staffml-validate-dev.yml does not run a lint step, so npm run lint has apparently been broken for any contributor since that bump landed.

Fix

  • Pinned eslint back to 9.39.5 (latest 9.x; also the version npm was already resolving as the nested peer dependency during npm ci, per the ERESOLVE warnings).
  • With lint actually running again, it surfaced three real errors, all fixed here:
    • lib/hooks/useFullQuestion.ts: summaryRef.current = summary was written directly in the render body, which the react-hooks/refs rule (part of eslint-config-next 16's hook-purity rules) flags as unsafe — writing to a ref during render can leave stale values behind an interrupted or Strict-Mode-double-invoked render. Moved the sync into a useLayoutEffect, which still runs before the fetch effect on the same commit (so the ref is current when read) without mutating a ref mid-render.
    • lib/plans.ts, lib/progress.ts: both had a require() call reaching into a sibling module (./progress, ./corpus) that has no circular dependency on the caller — confirmed neither corpus.ts nor progress.ts imports back into plans.ts. The plans.ts one was also silently swallowed by an unrelated try/catch, meaning any failure there would leave todayCompleted at 0 with no signal. Replaced both with static top-level imports.

Verification

  • npx eslint . — was crashing (TypeError, exit 2); now runs clean at 0 errors (94 pre-existing style warnings remain, out of scope here).
  • npx tsc --noEmit — clean, no new type errors.
  • npx vitest run — 24 test files / 131 tests, all passing.
  • npm run build — production static export succeeds, all 15 routes prerendered.

interviews/staffml/package.json pinned eslint to 10.4.0 (bumped from
9.39.4), but eslint-config-next 16.2.6's nested eslint-plugin-react
only supports eslint through ^9.7. Running `npm run lint` crashed
outright with "TypeError: contextOrFilename.getFilename is not a
function" instead of reporting lint results. CI never caught this
since staffml-validate-dev.yml does not run a lint step, so this went
unnoticed for anyone who bumped ESLint but never ran lint locally.

Pinned eslint back to 9.39.5 (latest 9.x, already resolved as the
nested peer dependency version). With lint actually running, it
surfaced three real errors:

- lib/hooks/useFullQuestion.ts: summaryRef.current was written
  directly in the render body. Moved the sync into a useLayoutEffect,
  which still runs before the fetch effect on the same commit (so the
  ref is current when read) without mutating a ref mid-render.
- lib/plans.ts, lib/progress.ts: require() calls for sibling modules
  that have no circular dependency on the caller, so they were
  unnecessary and swallowed by an unrelated try/catch in plans.ts.
  Replaced with static imports.
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

🎉 Thanks for contributing to StaffML!

We appreciate you sharing your knowledge. A maintainer will review the math and logic shortly.

P.S. If you haven't already, please drop a ⭐ on the repository!

@github-actions github-actions Bot added area: staffml Path interviews/ — auto-label; StaffML javascript Pull requests that update javascript code type: bug bug in rendering labels Aug 1, 2026
@Shashank-Tripathi-07 Shashank-Tripathi-07 self-assigned this Aug 6, 2026
@Shashank-Tripathi-07

Shashank-Tripathi-07 commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Without lint actually running, this repo had zero automated defense against this whole class of bug for as long as the eslint config stayed broken, since CI doesn't run lint either. It's not about style nitpicks here, it's the only check in the whole pipeline built to catch unsafe patterns like ref-mutation-during-render before a user hits the specific timing that turns it into a real, visible bug. eslint is important :)

@profvjreddi
profvjreddi merged commit 161f74d into harvard-edge:dev Aug 10, 2026
9 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Thanks @Shashank-Tripathi-07! 🎉

I added @Shashank-Tripathi-07 to staffml for: bug, code, test, tool.

The contributor tables are now handled directly by this workflow; no follow-up command is needed.

@Shashank-Tripathi-07
Shashank-Tripathi-07 deleted the fix/staffml-eslint-10-incompatible-with-eslint-config-next branch August 10, 2026 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: staffml Path interviews/ — auto-label; StaffML javascript Pull requests that update javascript code type: bug bug in rendering

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants