Skip to content

Conversation

@tonibardina
Copy link

@tonibardina tonibardina commented Oct 22, 2025

βœ… Checklist

  • I have followed every step in the contributing guide
  • The PR title follows the convention
  • I ran and verified the fix works as expected

πŸ§ͺ Testing

I ran the project locally and confirmed that infinite re-renders are no longer happening.
The subscription now initializes once and remains stable, even when using tag arrays.


🧾 Changelog

Fixed an issue causing infinite re-renders by ensuring the tag dependency in the useRealtimeRunsWithTag hook is stable.
React was previously interpreting a new array reference each render (due to shallow comparison), which triggered unnecessary re-subscriptions.


πŸ“Έ Screenshots

No visual changes β€” purely internal stability improvement.

πŸ’―

fixes #2629

@changeset-bot
Copy link

changeset-bot bot commented Oct 22, 2025

⚠️ No Changeset found

Latest commit: 49d6920

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 22, 2025

Walkthrough

The change modifies the useRealtimeRunsWithTag hook to normalize tag dependencies by introducing a stableTag variable that sorts and stringifies array-format tags, while preserving non-array tags unchanged. This stableTag replaces the original tag in the hook's effect dependency array and cleanup trigger points, preventing unnecessary re-executions when tag arrays contain identical elements in different orders.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

βœ… Passed checks (3 passed)
Check name Status Explanation
Title Check βœ… Passed The title "fix: avoid multiple rerenders when tags is an array" directly and clearly describes the main change documented in the raw summary: fixing an issue where tag arrays triggered unnecessary re-renders by introducing stable tag dependency tracking. The title uses standard commit convention format, is concise and specific, and a teammate reviewing history would immediately understand that this fixes a re-render performance issue related to array tags. The title is fully related to the core objective of the changeset.
Description Check βœ… Passed The pull request description is mostly complete and comprehensive, including a fully completed checklist confirming adherence to the contributing guide and PR title convention, a detailed Testing section describing local verification that re-renders no longer occur, a clear Changelog section explaining the fix and its root cause, and an appropriate Screenshots section noting no visual changes. The description is well-structured and provides sufficient context for reviewers to understand the change. The only missing element from the template is the "Closes #" line at the beginning, which would link the PR to a specific GitHub issue but is not critical given the clear description of the problem and solution.
Docstring Coverage βœ… Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • πŸ“ Generate docstrings
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❀️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
packages/react-hooks/src/hooks/useRealtime.ts (2)

431-433: Stable tag normalization looks good!

The approach of sorting and stringifying array tags creates a deterministic dependency key that prevents re-renders when tag order changes. For non-array tags, the value passes through unchanged.

Optional: Consider adding a comment for clarity.

A brief comment explaining why stableTag is needed would help future maintainers understand this pattern:

+  // Normalize array tags to prevent re-renders when order changes.
+  // Arrays with the same elements in different orders produce the same stableTag.
   const stableTag = Array.isArray(tag)
     ? JSON.stringify([...tag].sort())
     : tag;

434-465: Consider including tag in the dependency array to avoid stale closures.

The callback uses tag at line 444 but doesn't include it in the dependency array. While the current implementation works (assuming the API treats tag arrays as unordered sets), including tag in the dependency array would follow React best practices and avoid potential stale closure issues.

Apply this change to include tag in the dependency array:

-  }, [stableTag, mutateRuns, runsRef, abortControllerRef, apiClient, setError]);
+  }, [tag, stableTag, mutateRuns, runsRef, abortControllerRef, apiClient, setError]);

How this works together:

  • triggerRequest gets recreated whenever tag changes (by reference), ensuring it always has the current value
  • The effect (line 477) still uses stableTag, so it only runs when the set of tags changes, not when the order changes
  • This prevents infinite re-renders while avoiding stale closures
πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 3157b65 and d1a7742.

πŸ“’ Files selected for processing (1)
  • packages/react-hooks/src/hooks/useRealtime.ts (3 hunks)
🧰 Additional context used
πŸ““ Path-based instructions (1)
**/*.{ts,tsx}

πŸ“„ CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations

Files:

  • packages/react-hooks/src/hooks/useRealtime.ts
πŸ”‡ Additional comments (1)
packages/react-hooks/src/hooks/useRealtime.ts (1)

467-477: Excellent fix for the infinite re-render issue!

Using stableTag instead of tag in the effect dependency array is the key change that prevents unnecessary re-subscriptions when tag arrays change by reference but contain the same elements (possibly in different order). The effect will only re-run when the actual set of tags changes, not when the array reference changes.

@tonibardina tonibardina changed the title fix: avoid multiple rerenders when tags is an array fix 2629: avoid multiple rerenders when tags is an array Oct 23, 2025
@tonibardina tonibardina changed the title fix 2629: avoid multiple rerenders when tags is an array fix #2629: avoid multiple rerenders when tags is an array Oct 23, 2025
@tonibardina
Copy link
Author

tonibardina commented Oct 23, 2025

NOTE: temporary fixed it by memoizing my tags. Altho it works I dont think it should be the real fix as it is forcing your users to unnecessarily memoize their tags

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Infinite re-renders when using react hooks with tags

1 participant