fix: avoid retaining detached style containers - #818
justonemorenight wants to merge 1 commit into
Conversation
|
@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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. Walkthrough本次修改将 Changes容器缓存与 ShadowRoot 测试
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to 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)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
Summary
Fixes a memory leak where
dynamicCSS's module-levelcontainerCacheretains detached style containers (such as miniappShadowRoots or dynamic container elements) across component / micro-app lifecycles.Root Cause
src/Dom/dynamicCSS.tscurrently stores container mappings in a module-level strongMap:When an application or component rendered inside a
ShadowRoot(e.g., in micro-frontend architectures or isolated widgets) is unmounted and disposed, the detachedShadowRootremains strongly referenced as a key incontainerCache.Because the
ShadowRootcannot 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
ContainerType = Element | ShadowRoot)..get(container)and.set(container, parentNode). There is no key enumeration,.sizecheck, or iteration anywhere in the codebase.WeakMapallows the JavaScript engine to garbage collect the detached root and its subtree without requiring explicit per-app unmount hooks.WeakMapnaturally bounds lifetime per container without cross-app interference.clearContainerCache()support: Replaces thecontainerCacheinstance (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:
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
injectCSS,updateCSS,removeCSS,clearContainerCache) are strictly preserved.tests/dynamicCSS.test.tsxverifying ShadowRoot styling, updates, and cache reset without depending on non-deterministic GC timing.es/orlib/files are touched.Compatibility & Backport Question
mastercurrently publishes@rc-component/util. Many enterprise ecosystems and dependencies also consume the legacyrc-utilline (e.g.5.44.xon the5.xbranch). Would the maintainers be open to backporting this one-line fix to the5.xbranch for a patch release?Summary by CodeRabbit
Bug 修复
测试