chore(webapp): scope component effect synchronization - #4727
Conversation
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // Reset feedback state when conversation is reset | ||
| useEffect(() => { | ||
| if (conversation.length === 0) { | ||
| // oxlint-disable-next-line react/react-compiler -- This effect intentionally synchronizes local state after an external or lifecycle change. |
There was a problem hiding this comment.
🔍 Suppressed rule react/react-compiler does not appear to be enabled in the oxlint config
The PR adds ~40 // oxlint-disable-next-line react/react-compiler comments, but .oxlintrc.json never enables react/react-compiler (the config lists explicit react/* rules and only enables the correctness category). If the rule isn't active, every directive added here is a no-op (and would be reported as an unused directive if --report-unused-directives is ever turned on). Worth confirming whether the rule is enabled implicitly by the oxlint version in use, or whether a config change enabling it is missing from this PR.
Was this helpful? React with 👍 or 👎 to provide feedback.
| useEffect(() => { | ||
| // If disabled or no events | ||
| if (!enabled || streamedEvents === null) { | ||
| // oxlint-disable-next-line react/react-compiler -- This effect intentionally synchronizes local state after an external or lifecycle change. |
There was a problem hiding this comment.
🔍 Only the first setState in multi-setState effects is suppressed
Several suppressed effects contain more than one setState call, but only the first is annotated. Here, setIsConnected(undefined) on the early-return path is suppressed while setIsConnected(true/false) at apps/webapp/app/components/DevPresence.tsx:66-77 are not. The same shape appears in apps/webapp/app/components/billing/BillingAlertsSection.tsx:193-195 (setThresholdRows suppressed, setEmailValues not) and apps/webapp/app/components/integrations/VercelOnboardingModal.tsx:266-268. If the rule reports per-call rather than per-effect, lint will still fail on the unannotated calls; if it reports once per effect, the placement is fine but inconsistent across the codebase.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Scopes React Compiler diagnostics for component and hook effects that intentionally synchronize with navigation, submissions, browser APIs, streams, timers, or authoritative server values. Each suppression stays on the reported synchronization call rather than disabling analysis for the component.