Skip to content

Conversation

megboehlert
Copy link
Contributor

No description provided.

@Copilot Copilot AI review requested due to automatic review settings October 2, 2025 21:40
Copy link

changeset-bot bot commented Oct 2, 2025

🦋 Changeset detected

Latest commit: 9cb6057

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 19 packages
Name Type
rrweb Patch
rrweb-snapshot Patch
rrdom Patch
rrdom-nodejs Patch
rrweb-player Patch
@rrweb/all Patch
@rrweb/replay Patch
@rrweb/record Patch
@rrweb/types Patch
@rrweb/packer Patch
@rrweb/utils Patch
@rrweb/web-extension Patch
rrvideo Patch
@rrweb/rrweb-plugin-console-record Patch
@rrweb/rrweb-plugin-console-replay Patch
@rrweb/rrweb-plugin-sequential-id-record Patch
@rrweb/rrweb-plugin-sequential-id-replay Patch
@rrweb/rrweb-plugin-canvas-webrtc-record Patch
@rrweb/rrweb-plugin-canvas-webrtc-replay Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds cleanup logic to remove iframe window references from crossOriginIframeMap on iframe unload to address potential memory leaks.

  • Wraps existing message listener registration in braces and adds an unload event listener.
  • Introduces a changeset entry documenting the patch.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/rrweb/src/record/iframe-manager.ts Adds unload listener to delete iframe window from tracking map.
.changeset/shy-carrots-deny.md Records patch change note about memory leak fix.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 86 to 88
iframeEl.contentWindow?.addEventListener('unload', () => {
this.crossOriginIframeMap.delete(iframeEl.contentWindow!);
});
Copy link
Preview

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unload handler deletes using iframeEl.contentWindow at execution time, which may already point to a new Window after a navigation, leaving the original (unloaded) window object still stored in crossOriginIframeMap. Capture the original window reference before registering the listener and delete that specific reference instead.

Suggested change
iframeEl.contentWindow?.addEventListener('unload', () => {
this.crossOriginIframeMap.delete(iframeEl.contentWindow!);
});
if (iframeEl.contentWindow) {
const win = iframeEl.contentWindow;
win.addEventListener('unload', () => {
this.crossOriginIframeMap.delete(win);
});
}

Copilot uses AI. Check for mistakes.

@megboehlert megboehlert force-pushed the meg-fix-iframe-mem-leak branch from 13d72bd to 9cb6057 Compare October 3, 2025 17:52
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