fix: White screen after deleting a tag

[katsyuta]

Closes: #339

Problem:
After deleting a tag the tags list in Redux is updated, triggering a new render. But the tags tree is rebuilt later in useEffect, so VirtualTagsList renders using an old tree that still contains the deleted tag. As a result, the component tries to load data for the deleted item by looking up its id in the tags list, but the tag not exists, so the lookup returns undefined

Solution:
Rebuild the tags tree synchronously before rendering the child component so it always renders with the latest tree state

Summary by CodeRabbit

Summary by CodeRabbit

• Bug Fixes
  • Improved the Tags panel so the tag tree updates immediately when tag data changes.
  • Resolved cases where the displayed tree could lag behind the latest tags after the list was updated.
  • Ensured refreshed tag information is reflected consistently during screen updates, providing a more accurate and responsive browsing experience.

[coderabbitai]

Important

Review available on request

• 🔍 Trigger review

Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment @coderabbitai review to review the latest changes. For a full review, comment @coderabbitai full review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5b094e62-ce74-43d9-8e80-714ab4daa3e1

📝 Walkthrough

Walkthrough

TagsTree.tsx now rebuilds the tree synchronously during render when the tags reference changes. The previous post-render useEffect trigger was removed.

Changes

Tag Tree Rebuild Fix

Layer / File(s)	Summary
In-render tree rebuild packages/app/src/features/MainScreen/TagsPanel/TagsTree.tsx	Tracks the previous tags reference and calls tree.rebuildTree() synchronously when the reference changes. Removes the unused useEffect import and the previous effect-based rebuild.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: vitonsky

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The change rebuilds the tags tree before child rendering and directly addresses issue #339.
Out of Scope Changes check	✅ Passed	The pull request changes only the tags tree rebuild behavior required to fix the linked issue.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the primary bug fix: preventing a white screen after deleting a tag.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[vitonsky]

@katsyuta could you update pull request description to explain in one paragraph what is the root cause of the problem and how this changes fix that problem.

[katsyuta]

@vitonsky, description is ready

[vitonsky]

How it does work?

1. How to trigger first tree build? At first render this condition will not be passed
2. Why not to use useMemo?
3. Originally the deps is [tags, tree], why we depend on tags only now?

[katsyuta]

1. tree.rebuildTree() is only needed when the source data changes. The initial tree build is triggered inside useTree with the actual data
2. I don't now how useMemo would help here, the only requirement is to ensure that rebuild is called after tags change and before child components are rendered
3. As i understand, tree was in the dependencies because it was just an esLint requirement. tree is a stable instance created by useTree, the rebuild is only required when the data is changed

[vitonsky]

Are you sure rebuildTree can be called while rendering?

In case that call would start another render we may have problem, because such logic must not be run while rendering. Why this logic is wrapped via useEffect for now? Maybe just to run it after rendering, i don't know. Check it.

I don't now how useMemo would help here, the only requirement is to ensure that rebuild is called after tags change and before child components are rendered

React components must looks declarative. Don't write imperative code in component body. Invent some hook for that logic or search for NPM.

[katsyuta]

Calling rebuildTree trigger a re-render, but with the guard it is safe - no infinite loop.
When state updates during render, React re-renders that same component immediately, before rendering its children, within the same render cycle.
I wrap this logic to hook

[vitonsky]

@katsyuta as I understand, the re-render call while run another render is unsafe and can throw errors by itself.

The problem is not in infinite loop. I saw errors about that, but I don't remember exactly why that's bad. Research that

[katsyuta]

In the react docs, they write about the pattern that i suggested. React considers it a rare pattern and recommends not overusing it, but it is valid as long as we follow two rules:

• Update only the state of the component that is currently rendering
• Use guard to avoid an infinite loop

If the state belonging to the component itself is updated, no error will occur. useTree uses a state to trigger a render, so when rebuildTree is called we update the state owned by the component itself

As i understand, the error can occur only if update a different component while rendering. I found a some issue about this.