chore(webapp): scope intentional draft synchronization - #4725
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}
|
|
||
| // On first render | ||
| useEffect(() => { | ||
| // oxlint-disable-next-line react/react-compiler -- A changed date intentionally resets the timer-backed relative text. |
There was a problem hiding this comment.
🔍 Suppression comments sit inside the effect callback, not on the useEffect call
Each added directive is placed as the first statement inside the effect callback so it suppresses the setX(...) line. If the react-compiler diagnostic is instead reported at the useEffect( call expression (which is where compiler-related bail-outs are usually attributed), these oxlint-disable-next-line comments would not suppress anything and lint would still fail. Note also that react/react-compiler is not listed in .oxlintrc.json rules or plugins (only typescript, import, react, jsx-a11y plugins with the correctness category), and oxlint is not installed in the workspace here, so I could not verify locally that these directives take effect (or that they are even needed). Worth confirming with pnpm run lint in CI that each directive actually suppresses a reported diagnostic and none is a no-op.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // oxlint-disable-next-line react/react-compiler -- Deployment status changes intentionally reset the user-controlled collapse state. | ||
| setCollapsed(initialCollapsed); |
There was a problem hiding this comment.
🔍 Comment describes behavior that may itself be undesirable
The justification claims deployment status changes "intentionally reset the user-controlled collapse state". Because initialCollapsed is derived from the deployment's live status, a user who manually expands/collapses the log panel will have that choice overwritten whenever the streamed status changes. The suppression documents this as intentional, but the underlying UX behavior (losing the user's manual toggle mid-deployment) looks accidental and may be worth revisiting rather than blessing.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Scopes state synchronization that intentionally resets editable drafts from authoritative server values, deployment state, or programmatic filter changes. These values cannot be derived during render without removing user control between resets.