Skip to content

Conversation

@pauldambra
Copy link
Contributor

noticed this while doing something else
this call to console.warn is direct so it doesn't obey the provided config
let's make it play nicely

Copilot AI review requested due to automatic review settings July 1, 2025 13:52
@changeset-bot
Copy link

changeset-bot bot commented Jul 1, 2025

🦋 Changeset detected

Latest commit: 5a1cb93

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

This PR includes changesets to release 19 packages
Name Type
rrweb-snapshot Patch
rrweb 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 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

This PR ensures empty CDATA nodes are safely skipped and that warning messages respect the configured logger.

  • Swap direct console.warn calls in Replayer to use this.warn so messages obey the provided config.
  • Add a guard in buildNode to return null instead of throwing on empty CDATA content.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/rrweb/src/replay/index.ts Replaced console.warn(e) with this.warn(e) in the replay loop
packages/rrweb-snapshot/src/rebuild.ts Added if (n.textContent.trim() === '') return null for CDATA
Comments suppressed due to low confidence (3)

packages/rrweb-snapshot/src/rebuild.ts:435

  • Add a unit test case for empty CDATA nodes to verify that this guard prevents exceptions and returns null as intended.
      if (n.textContent.trim() === '') {

packages/rrweb-snapshot/src/rebuild.ts:435

  • Document this new behavior in the function's JSDoc or comments, clarifying that empty CDATA sections will now be skipped by returning null.
      if (n.textContent.trim() === '') {

packages/rrweb-snapshot/src/rebuild.ts:435

  • Introducing a null return here may violate the function's expected return type and could cause null dereference errors downstream. Consider updating the return type signature or ensuring callers handle null.
      if (n.textContent.trim() === '') {

@pauldambra pauldambra changed the title fix: do not throw on empty cdata fix: move a console log to the warn method so it can be suppressed Jul 1, 2025
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.

3 participants