feat: add prefer-use-sync-external-store rule#154
Draft
Conversation
New rule (severity: warn) flagging the §11 anti-pattern from React's
'You Might Not Need an Effect' guide:
const [snapshot, setSnapshot] = useState(getSnapshot());
useEffect(() => {
const unsub = store.subscribe(() => setSnapshot(getSnapshot()));
return unsub;
}, []);
The hand-rolled subscribe pattern reimplements useSyncExternalStore in
user space — incorrectly. The hook handles tearing during concurrent
renders and SSR snapshots; the manual pattern doesn't. The rule
suggests:
const snapshot = useSyncExternalStore(store.subscribe, getSnapshot);
Detector requires a four-vertex AST match before firing:
(1) useEffect with empty deps `[]`
(2) body declares `const u = X.subscribe(handler)` OR
directly invokes a subscription method X.addEventListener(...)
(3) cleanup is a `return` that returns the unsubscribe binding
directly OR returns a closure that unsubscribes
(4) handler is a single `setY(<getter>)` whose <getter> is
structurally equal to the matching useState's initializer
Subscription method names recognized: subscribe, addEventListener,
addListener, on, watch, listen, sub. Covers zustand, Redux vanilla,
Valtio, Effector, Jotai store API, RxJS observables, and the browser
`addEventListener` shape (matchMedia, navigator.onLine).
Resolves Identifier-passed handlers like
const onChange = () => setIsOnline(navigator.onLine);
window.addEventListener('online', onChange);
by walking earlier const declarations in the same effect body.
Adds:
- SUBSCRIPTION_METHOD_NAMES + UNSUBSCRIPTION_METHOD_NAMES to constants
(replaces the local 4-element set in advancedEventHandlerRefs;
same shape, broader allowlist).
- areExpressionsStructurallyEqual helper for the four-vertex
structural match.
Tests: 7 regression cases — canonical store-subscribe shape, browser
addEventListener shape, lazy-initializer variant, and four
no-flag cases (legitimate chat connection with non-empty deps,
subscribe-as-side-effect with no useState pair, mismatched setter
arg, missing cleanup).
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
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
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.
New rule (severity:
warn) flagging the §11 anti-pattern from React's You Might Not Need an Effect guide.What it catches
The hand-rolled subscribe pattern reimplements
useSyncExternalStorein user space — incorrectly. The hook handles tearing during concurrent renders and SSR snapshots; the manual pattern doesn't. The rule suggests:Detector — four-vertex AST match
Real-world false positives are essentially impossible because all four conditions must coincide:
useEffectwith empty deps[]const u = X.subscribe(handler)OR directly invokes a subscription methodX.addEventListener(...)returnthat returns the unsubscribe binding directly OR returns a closure that unsubscribessetY(<getter>)whose<getter>is structurally equal to the matchinguseState's initializerSubscription method names recognized:
subscribe,addEventListener,addListener,on,watch,listen,sub— covers zustand, Redux vanilla, Valtio, Effector, Jotai store API, RxJS observables, and the browseraddEventListenershape (matchMedia,navigator.onLine).The detector also resolves Identifier-passed handlers (the dominant real-world shape):
Library safety
Modern store hooks (
useAtomValuefrom Jotai,useStorefrom zustand v4+,useSelectorfrom react-redux v8+) already useuseSyncExternalStoreunder the hood and are not flagged — the rule only catches code that bypasses the library's hook to roll its own subscription.Tests — 7 regression cases
store.subscribeshape (article §11)addEventListenershape (navigator.onLine)useState(() => getSnapshot())useState(audit-only)useStateinitializerPlus a smoke test in
run-oxlint.test.tsagainst a fixture component.Refactor included
SUBSCRIPTION_METHOD_NAMESandUNSUBSCRIPTION_METHOD_NAMESsets inconstants.ts(replaces the local 4-element set inadvancedEventHandlerRefs— same shape, broader allowlist).areExpressionsStructurallyEqualhelper inhelpers.tsfor the four-vertex match.Checks
488/488 tests passing locally. Lint, typecheck, format clean. Changeset included (minor bump).