Conversation
📝 WalkthroughWalkthroughCentralizes React root lifecycle for post-renderer enhancers (track/unmount roots and change enhancers to return Root[]), adds maxPages caps and tighter next-page guards across many infinite query options, and applies several UI/behavior fixes (tooltip/interval/key handling, timeout cleanup, disabling vote buttons, wallet token list tweak, version bumps). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
feruzm
added
the
patch
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/_components/market-data/market.tsx (1)
65-71:⚠️ Potential issue | 🟠 MajorBroken tooltip: arrow function defeats Highcharts'
this-based formatter context.Highcharts invokes
tooltip.formatterwiththisbound to the hovered Point instance (Highcharts v12+; for shared/split tooltips,this.pointsis also available). Arrow functions capture the lexicalthisand ignore.call()rebinding, so no context is received.The current code uses an arrow function expecting a
{ chart }parameter, but Highcharts passes no explicit arguments. At runtime,chartisundefined, andchart.hoverPoint.options.ximmediately throwsTypeError: Cannot read property 'hoverPoint' of undefined. Theas anycast masks this type error from the compiler.Replace with a regular function to access
this.xandthis.ydirectly from the Point context:🛠️ Proposed fix
- formatter: (({ chart }: any) => { - let date = dayjs(chart.hoverPoint.options.x).calendar(); - let rate = chart.hoverPoint.options.y; - return `<div><div>${i18next.t("g.when")}: <b>${date}</b></div><div>${i18next.t( - "g.price" - )}:<b>${rate.toFixed(3)}</b></div></div>`; - }) as any, + formatter: function (this: Highcharts.TooltipFormatterContextObject) { + const date = dayjs(this.x).calendar(); + const rate = this.y as number; + return `<div><div>${i18next.t("g.when")}: <b>${date}</b></div><div>${i18next.t( + "g.price" + )}:<b>${rate.toFixed(3)}</b></div></div>`; + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/_components/market-data/market.tsx` around lines 65 - 71, The tooltip.formatter is currently an arrow function (formatter) that expects a { chart } param, which breaks Highcharts' this-based formatter context; change formatter to a regular function so it can use the Point context (this) directly: read date from dayjs(this.x).calendar() and rate from this.y (then call toFixed(3)) and keep i18next.t("g.when") / i18next.t("g.price") for labels; remove the ({ chart }: any) parameter and the arrow so Highcharts' .call(thisPoint) works and the TypeError disappears.
♻️ Duplicate comments (1)
apps/web/src/features/post-renderer/components/extensions/hive-operation-extension.tsx (1)
110-113: LGTM —for...ofcleanup correctly unmounts all tracked roots.The past Biome lint error (
forEachreturning a value) is resolved. Thefor...ofloop avoids the implicit return, and settingrootsRef.current = []ensures the accumulator is cleared before the next effect run.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/post-renderer/components/extensions/hive-operation-extension.tsx` around lines 110 - 113, The cleanup function is already fixed: replace any prior use of Array.prototype.forEach that returned a value with a for...of loop that calls r.unmount() for each tracked root and then reset rootsRef.current = []; ensure the cleanup return in the effect uses the for...of iteration over rootsRef.current and the explicit rootsRef.current = [] reset so all roots (via their unmount method) are unmounted and the accumulator cleared.
🧹 Nitpick comments (1)
apps/web/src/features/post-renderer/components/extensions/hive-operation-extension.tsx (1)
95-99: Consider capturing the listener reference for explicit removal during cleanup.
container.addEventListener("click", () => onClickRef.current?.(op))adds an anonymous listener that can never be removed. SincecontainerRefis a stableRefObjectthe effect effectively runs once, so in the normal unmount path the entire parent DOM subtree is torn down and GC handles it — but if the effect ever re-runs (e.g., a differentcontainerRefobject is passed), old containers remain in the DOM with zombie click handlers.Capturing the handler and removing it in the cleanup would make the lifecycle explicit and guard against that edge case:
🔧 Proposed refactor
- container.addEventListener("click", () => onClickRef.current?.(op)); + const clickHandler = () => onClickRef.current?.(op); + container.addEventListener("click", clickHandler);Then store
{ root, container, clickHandler }(or a parallelhandlersRef) so the cleanup can callcontainer.removeEventListener("click", clickHandler)beforer.unmount().Alternatively, keep the approach but add a reset at the start of the effect body so stale containers are removed from the DOM before re-processing:
useEffect(() => { + for (const r of rootsRef.current) { r.unmount(); } + rootsRef.current = []; Array.from(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/post-renderer/components/extensions/hive-operation-extension.tsx` around lines 95 - 99, The click listener added via container.addEventListener("click", () => onClickRef.current?.(op)) uses an anonymous function that cannot be removed; change the effect to create a named clickHandler (e.g., const clickHandler = () => onClickRef.current?.(op)), attach it with container.addEventListener("click", clickHandler), and store the triple { root, container, clickHandler } (or maintain a parallel handlersRef) instead of only pushing root to rootsRef; in the effect cleanup iterate these stored entries and call container.removeEventListener("click", clickHandler) before calling root.unmount() so old containers cannot retain zombie handlers if the effect re-runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/src/app/_components/market-data/market.tsx`:
- Around line 65-71: The tooltip.formatter is currently an arrow function
(formatter) that expects a { chart } param, which breaks Highcharts' this-based
formatter context; change formatter to a regular function so it can use the
Point context (this) directly: read date from dayjs(this.x).calendar() and rate
from this.y (then call toFixed(3)) and keep i18next.t("g.when") /
i18next.t("g.price") for labels; remove the ({ chart }: any) parameter and the
arrow so Highcharts' .call(thisPoint) works and the TypeError disappears.
---
Duplicate comments:
In
`@apps/web/src/features/post-renderer/components/extensions/hive-operation-extension.tsx`:
- Around line 110-113: The cleanup function is already fixed: replace any prior
use of Array.prototype.forEach that returned a value with a for...of loop that
calls r.unmount() for each tracked root and then reset rootsRef.current = [];
ensure the cleanup return in the effect uses the for...of iteration over
rootsRef.current and the explicit rootsRef.current = [] reset so all roots (via
their unmount method) are unmounted and the accumulator cleared.
---
Nitpick comments:
In
`@apps/web/src/features/post-renderer/components/extensions/hive-operation-extension.tsx`:
- Around line 95-99: The click listener added via
container.addEventListener("click", () => onClickRef.current?.(op)) uses an
anonymous function that cannot be removed; change the effect to create a named
clickHandler (e.g., const clickHandler = () => onClickRef.current?.(op)), attach
it with container.addEventListener("click", clickHandler), and store the triple
{ root, container, clickHandler } (or maintain a parallel handlersRef) instead
of only pushing root to rootsRef; in the effect cleanup iterate these stored
entries and call container.removeEventListener("click", clickHandler) before
calling root.unmount() so old containers cannot retain zombie handlers if the
effect re-runs.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
packages/sdk/dist/browser/index.jsis excluded by!**/dist/**packages/sdk/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.cjsis excluded by!**/dist/**packages/sdk/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.mjsis excluded by!**/dist/**packages/sdk/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (4)
apps/web/src/app/_components/market-data/market.tsxapps/web/src/features/post-renderer/components/extensions/hive-operation-extension.tsxapps/web/src/features/post-renderer/components/extensions/youtube-video-extension.tsxpackages/sdk/src/modules/hive-engine/queries/get-hive-engine-token-transactions-query-options.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/features/post-renderer/components/extensions/youtube-video-extension.tsx
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Performance
Chores