Fix Memory Leak in SocialShareButton by Removing Event Listeners on D… #10
Conversation
…estroy Updated version number and added event listener tracking.
📝 WalkthroughWalkthroughInternal refactoring of the SocialShareButton class improves event listener tracking, consolidates URL/message construction logic, streamlines modal and copy operations, and enhances cleanup procedures. Version bumped to 1.0.4. Public API surface remains unchanged. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/social-share-button.js (1)
131-138: Hardcoded promotional messages may be unexpected for library users.The share messages now include opinionated text like "Clean UI, smooth flow — worth peeking" and "Clean stuff, take a look 👇". Library users typically expect the share content to reflect their own messaging, not preset promotional copy.
Consider making these message templates configurable through options, or keeping the messages neutral (just title + description + URL).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/social-share-button.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kpj2006
Repo: AOSSIE-Org/SocialShareButton PR: 1
File: src/social-share-button.js:10-11
Timestamp: 2025-12-21T12:11:23.512Z
Learning: SSR compatibility is important for the SocialShareButton library (src/social-share-button.js) since it's designed for public library usage across multiple frameworks including SSR environments.
📚 Learning: 2025-12-21T12:11:14.980Z
Learnt from: kpj2006
Repo: AOSSIE-Org/SocialShareButton PR: 1
File: src/social-share-button.js:10-11
Timestamp: 2025-12-21T12:11:14.980Z
Learning: SSR compatibility is important for public libraries. For files under src/, ensure the SocialShareButton (and related public APIs) render correctly in SSR environments across frameworks. Check for browser-only globals, conditional code paths, correct hydration behavior, and that the library exports behave consistently when used in SSR.
Applied to files:
src/social-share-button.js
🧬 Code graph analysis (1)
src/social-share-button.js (1)
src/social-share-button-react.jsx (2)
SocialShareButton(3-77)SocialShareButton(3-77)
🔇 Additional comments (5)
src/social-share-button.js (5)
32-32: Good addition for listener tracking.Initializing
_listenersin the constructor is the right approach for tracking event listeners throughout the component lifecycle. This directly supports the memory leak fix objective.
144-180: Well-implemented event listener tracking.The refactored
attachEvents()correctly stores each listener as a{ element, type, handler }tuple in_listeners. This is the proper pattern for tracking and later removing event listeners. Key improvements:
- Named handlers instead of anonymous functions enable proper removal
- Document-level
keydownlistener (line 178-179) is tracked, preventing a common leak source- All modal-related listeners are tracked for complete cleanup
236-245: Properly implemented cleanup in destroy().The enhanced
destroy()method correctly:
- Iterates all tracked listeners and removes them with
removeEventListener- Clears the
_listenersarray- Removes DOM elements
- Resets page overflow
This directly addresses the memory leak issue described in the PR objectives.
247-265: LGTM on options update and color handling.The
updateOptions()method correctly syncs the URL to the modal input, and theapplyCustomColors()method maintains idempotent style tag creation. These changes align well with the React wrapper'supdateOptionsusage shown in the relevant code snippets.
268-274: SSR-compatible module exports maintained.The environment checks for
moduleandwindowensure the library works correctly in both server-side rendering and browser environments. Based on learnings, this SSR compatibility is important for the library.
| } catch { | ||
| copyBtn.textContent = 'Failed'; | ||
| setTimeout(() => { | ||
| copyBtn.textContent = 'Copy'; | ||
| }, 2000); | ||
| setTimeout(() => { copyBtn.textContent = 'Copy'; }, 2000); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "package.json" -type f | head -5Repository: AOSSIE-Org/SocialShareButton
Length of output: 86
🏁 Script executed:
cat package.jsonRepository: AOSSIE-Org/SocialShareButton
Length of output: 939
🏁 Script executed:
git ls-files | grep -E "(webpack|babel|tsconfig|\.browserslistrc)" | head -10Repository: AOSSIE-Org/SocialShareButton
Length of output: 54
🏁 Script executed:
cat -n src/social-share-button.js | sed -n '225,235p'Repository: AOSSIE-Org/SocialShareButton
Length of output: 561
Optional catch binding requires ES2019 support; library ships uncompiled.
The catch { syntax (ES2019 optional catch binding) is used without transpilation. Since package.json has no engines field specifying Node.js version requirements and the library ships source files directly without a build step, this syntax will fail in:
- Node.js < 10.0
- IE 11 and older browsers
- SSR environments using older Node versions
Either configure transpilation (Babel) or explicitly document the minimum supported versions (e.g., "engines": {"node": ">=10.0"}) if ES2019+ is intentional.
🤖 Prompt for AI Agents
In src/social-share-button.js around lines 230–233, the code uses the ES2019
optional catch binding syntax ("catch { ... }") which breaks in older
Node/browser runtimes because the package ships uncompiled; change the catch to
a standard form or ensure transpilation/documentation: either replace "catch {
... }" with "catch (err) { ... }" (and optionally use or log err) so it's
compatible without transpilation, or add a build/transpile step (Babel/TS) to
compile to ES5 and/or add an "engines" field in package.json (e.g., "engines":
{"node": ">=10.0"}) and document the minimum supported browser/Node versions if
you intend to keep ES2019 syntax.
|
Fixed memory leak in SocialShareButton by properly removing all event listeners on destroy(). |
|
If you want further changes then please reply on my PR |
1 participant
This PR fixes a memory leak in the SocialShareButton component by properly removing all event listeners when the component is destroyed. Previously, document-level and element-bound event listeners were not unregistered during destroy(), which could cause memory leaks and unexpected behavior in SPAs or applications where the component is mounted and unmounted frequently.
Changes Made:
Added a _listeners array to track all attached event listeners.
Updated attachEvents() to store each listener (element, event type, handler) in _listeners.
Updated destroy() to remove all tracked listeners before removing DOM elements.
Impact:
Prevents memory leaks in dynamic applications.
Ensures proper cleanup for modal, share buttons, copy button, and ESC key events.
Testing:
Verified that all sharing functionality works before and after component destruction.
Confirmed no event listeners remain attached after calling destroy().
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.