Skip to content

fix: avoid retaining detached style containers - #818

Open
justonemorenight wants to merge 1 commit into
react-component:masterfrom
justonemorenight:fix/dynamic-css-weakmap
Open

justonemorenight wants to merge 1 commit into
react-component:masterfrom
justonemorenight:fix/dynamic-css-weakmap

Conversation

@justonemorenight

@justonemorenight justonemorenight commented Sep 17, 2026

Copy link
Copy Markdown

Summary

Fixes a memory leak where dynamicCSS's module-level containerCache retains detached style containers (such as miniapp ShadowRoots or dynamic container elements) across component / micro-app lifecycles.

Root Cause

src/Dom/dynamicCSS.ts currently stores container mappings in a module-level strong Map:

const containerCache = new Map<ContainerType, Node & ParentNode>();

When an application or component rendered inside a ShadowRoot (e.g., in micro-frontend architectures or isolated widgets) is unmounted and disposed, the detached ShadowRoot remains strongly referenced as a key in containerCache.

Because the ShadowRoot cannot be garbage collected, it retains its entire descendant DOM tree as well as React delegated event listeners attached to those nodes. In workflows with repeated mount/unmount cycles, DOM nodes and listeners accumulate linearly over time.

Why WeakMap Is the Correct Solution

  • Object keys: Keys are always DOM objects (ContainerType = Element | ShadowRoot).
  • Minimal interface usage: The cache only uses .get(container) and .set(container, parentNode). There is no key enumeration, .size check, or iteration anywhere in the codebase.
  • Automatic reclamation: Once external references to a detached container/ShadowRoot are dropped, WeakMap allows the JavaScript engine to garbage collect the detached root and its subtree without requiring explicit per-app unmount hooks.
  • Isolated cache lifetime: Clearing a global cache on every unmount is unsafe when multiple active micro-apps share the same module instance. WeakMap naturally bounds lifetime per container without cross-app interference.
  • clearContainerCache() support: Replaces the containerCache instance (containerCache = new WeakMap<ContainerType, Node & ParentNode>()), preserving the exact existing behavior for test suites.

Measured Evidence & Profiling

In a real-world workload switching between an isolated Shadow DOM miniapp and a host application 30 times, taking snapshots after forced Chromium garbage collection:

Metric Before (strong Map) After (WeakMap) Run 2 (Plateau pass)
DOM nodes +7,047 +60 +34
Event listeners +9,008 0 -1
JS heap +8,048,764 B +5,052,640 B +2,586,176 B

Limitations: While this change eliminates the retention of detached style containers and their associated DOM/listener leak, it does not claim to eliminate all heap growth from independent application-level allocations.

Verification & Testing

  • Public API signatures and runtime behaviors (injectCSS, updateCSS, removeCSS, clearContainerCache) are strictly preserved.
  • Added non-flaky regression tests in tests/dynamicCSS.test.tsx verifying ShadowRoot styling, updates, and cache reset without depending on non-deterministic GC timing.
  • Only TypeScript source files are modified; no generated es/ or lib/ files are touched.

Compatibility & Backport Question

master currently publishes @rc-component/util. Many enterprise ecosystems and dependencies also consume the legacy rc-util line (e.g. 5.44.x on the 5.x branch). Would the maintainers be open to backporting this one-line fix to the 5.x branch for a patch release?

Summary by CodeRabbit

  • Bug 修复

    • 优化容器缓存管理,减少不再使用的容器缓存占用内存。
    • 修复缓存重置后样式更新无法正常工作的风险。
  • 测试

    • 增加对 ShadowRoot 中样式注入、更新、移除及缓存重置行为的覆盖。

@vercel

vercel Bot commented Sep 17, 2026

Copy link
Copy Markdown

@justonemorenight is attempting to deploy a commit to the afc163's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: b965d174-8708-4687-8a42-31a8b567fc13

📥 Commits

Reviewing files that changed from the base of the PR and between 389c571 and 0c56c0a.

📒 Files selected for processing (2)
  • src/Dom/dynamicCSS.ts
  • tests/dynamicCSS.test.tsx

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


Walkthrough

本次修改将 containerCache 改为 WeakMap,并通过重新赋值清空缓存。新增测试覆盖 ShadowRoot 中的样式注入、更新、缓存重置和移除。

Changes

容器缓存与 ShadowRoot 测试

Layer / File(s) Summary
WeakMap 缓存与 ShadowRoot 行为验证
src/Dom/dynamicCSS.ts, tests/dynamicCSS.test.tsx
containerCache 改用 WeakMapclearContainerCache() 创建新的空缓存。测试验证 ShadowRoot 中的样式注入、同一 key 更新、缓存重置后的更新,以及 removeCSS 的清理行为。

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 0c56c

The cache no longer strongly retains containers, while cache reset and ShadowRoot style operations remain covered by the changed tests. No current merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了主要变更:避免缓存保留已分离的样式容器。该目标与使用 WeakMap 修改 containerCache 的实现一致。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR

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

小兔捧来一枚新缓存
ShadowRoot 里样式安静生长
同一 key 原地换装
清空之后流程仍畅
removeCSS 让夜色清爽

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

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.

1 participant