From dce270e341932eca0e071b6f9282c120de1b8551 Mon Sep 17 00:00:00 2001 From: jonathanmos <48201295+jonathanmos@users.noreply.github.com> Date: Tue, 7 Jul 2026 16:45:36 +0300 Subject: [PATCH] Fix missing SVGs from barrel re-exports and unbuilt local SVG map --- .../src/cli/generate-sr-assets.ts | 7 ++ .../react-native-babel-plugin/src/index.ts | 8 +- .../src/libraries/react-native-svg/index.ts | 8 +- .../test/react-native-svg.test.ts | 104 ++++++++++++++++++ .../sessionreplay/mappers/SvgViewMapper.kt | 35 +++--- .../mappers/SvgViewMapperTest.kt | 14 ++- 6 files changed, 150 insertions(+), 26 deletions(-) diff --git a/packages/react-native-babel-plugin/src/cli/generate-sr-assets.ts b/packages/react-native-babel-plugin/src/cli/generate-sr-assets.ts index c6cffe00e..c9448d39f 100644 --- a/packages/react-native-babel-plugin/src/cli/generate-sr-assets.ts +++ b/packages/react-native-babel-plugin/src/cli/generate-sr-assets.ts @@ -6,6 +6,7 @@ */ import { transformSync } from '@babel/core'; +import * as babelTypes from '@babel/types'; import glob from 'fast-glob'; import fs from 'fs'; import path from 'path'; @@ -371,6 +372,12 @@ function generateSessionReplayAssets() { const reactNativeSVG = new ReactNativeSVG(rootDir, assetsPath, true); + // pre() skips its own setApiTypes/buildSvgMap call when __internal_reactNativeSVG + // is injected, so do it here up front instead — buildSvgMap() is a no-op until + // setApiTypes() has run. + reactNativeSVG.setApiTypes(babelTypes); + reactNativeSVG.buildSvgMap(); + for (const file of files) { try { const code = fs.readFileSync(file, 'utf8'); diff --git a/packages/react-native-babel-plugin/src/index.ts b/packages/react-native-babel-plugin/src/index.ts index 6c41836c5..4b2fa42c4 100644 --- a/packages/react-native-babel-plugin/src/index.ts +++ b/packages/react-native-babel-plugin/src/index.ts @@ -53,13 +53,19 @@ export default declare( assetsPath = getAssetsPath(); } - reactNativeSVG = options.__internal_reactNativeSVG; + // Reuse the instance across files in the same worker instead of + // rebuilding it (and rescanning) per file. + reactNativeSVG = + options.__internal_reactNativeSVG ?? reactNativeSVG; + if (!reactNativeSVG && assetsPath) { reactNativeSVG = new ReactNativeSVG( process.cwd(), assetsPath, options.__internal_saveSvgMapToDisk || false ); + // buildSvgMap() is a no-op until setApiTypes() has run. + reactNativeSVG.setApiTypes(api.types); reactNativeSVG.buildSvgMap(); } reactNativeSVG?.setApiTypes(api.types); diff --git a/packages/react-native-babel-plugin/src/libraries/react-native-svg/index.ts b/packages/react-native-babel-plugin/src/libraries/react-native-svg/index.ts index 1d3fed1ab..e24194b86 100644 --- a/packages/react-native-babel-plugin/src/libraries/react-native-svg/index.ts +++ b/packages/react-native-babel-plugin/src/libraries/react-native-svg/index.ts @@ -169,9 +169,15 @@ export class ReactNativeSVG { ); for (const spec of path.node.specifiers) { if (spec.type === 'ExportSpecifier') { + // spec.local.name is 'default' for `export { default as Logo }`; + // spec.exported is the name consumers actually import ('Logo'). + // It can be an Identifier or a string literal, so handle both. + const exported = spec.exported; const name = getNodeName( this.t, - spec.local.name + this.t.isStringLiteral(exported) + ? exported.value + : exported.name ); if (name) { this.localSvgMap[name] = { diff --git a/packages/react-native-babel-plugin/test/react-native-svg.test.ts b/packages/react-native-babel-plugin/test/react-native-svg.test.ts index 5427e9d76..5f8a362ca 100644 --- a/packages/react-native-babel-plugin/test/react-native-svg.test.ts +++ b/packages/react-native-babel-plugin/test/react-native-svg.test.ts @@ -827,3 +827,107 @@ describe('SessionReplayView.Privacy SVG Wrapper', () => { expect(output).not.toContain('style'); }); }); + +describe('ReactNativeSVG.buildSvgMap', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'dd-buildsvgmap-')); + // Write a minimal SVG file for imports to point at + fs.writeFileSync( + path.join(tmpDir, 'icon.svg'), + '' + ); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + // buildSvgMap requires setApiTypes to be called first (this.t guard). + it('should populate localSvgMap from a default import of an SVG file', () => { + const srcFile = path.join(tmpDir, 'Component.tsx'); + fs.writeFileSync( + srcFile, + `import Logo from './icon.svg';\nexport default function C() { return ; }` + ); + + const instance = new ReactNativeSVG(tmpDir, tmpDir, false); + instance.setApiTypes(t); // must come before buildSvgMap + instance.buildSvgMap(); + + expect(instance.localSvgMap['Logo']).toBeDefined(); + expect(instance.localSvgMap['Logo'].path).toBe( + path.join(tmpDir, 'icon.svg') + ); + }); + + it('should populate localSvgMap from a named import of an SVG file', () => { + const srcFile = path.join(tmpDir, 'Component.tsx'); + fs.writeFileSync( + srcFile, + `import { ReactComponent as StarIcon } from './icon.svg';\nexport default function C() { return ; }` + ); + + const instance = new ReactNativeSVG(tmpDir, tmpDir, false); + instance.setApiTypes(t); + instance.buildSvgMap(); + + expect(instance.localSvgMap['StarIcon']).toBeDefined(); + }); + + // spec.local.name is 'default' for `export { default as Logo }` — the exported + // name ('Logo') must be used as the map key instead. + it('should populate localSvgMap with the exported name for aliased re-exports', () => { + const barrelFile = path.join(tmpDir, 'icons.ts'); + fs.writeFileSync( + barrelFile, + `export { default as Logo } from './icon.svg';` + ); + + const instance = new ReactNativeSVG(tmpDir, tmpDir, false); + instance.setApiTypes(t); + instance.buildSvgMap(); + + // 'Logo' is what consumers import and use as — must be the key + expect(instance.localSvgMap['Logo']).toBeDefined(); + expect(instance.localSvgMap['Logo'].path).toBe( + path.join(tmpDir, 'icon.svg') + ); + // 'default' must NOT be stored — it matches nothing in user JSX + expect(instance.localSvgMap['default']).toBeUndefined(); + }); + + it('should populate localSvgMap with the exported name for non-aliased re-exports', () => { + const barrelFile = path.join(tmpDir, 'icons.ts'); + fs.writeFileSync(barrelFile, `export { StarIcon } from './icon.svg';`); + + const instance = new ReactNativeSVG(tmpDir, tmpDir, false); + instance.setApiTypes(t); + instance.buildSvgMap(); + + expect(instance.localSvgMap['StarIcon']).toBeDefined(); + }); + + // pre() must reuse the shared instance across files rather than rebuilding it + // (and rescanning) per file. + it('should not overwrite localSvgMap when buildSvgMap is called a second time on a fresh instance with saveSvgMapToDisk=false and no cache file', () => { + const srcFile = path.join(tmpDir, 'Component.tsx'); + fs.writeFileSync(srcFile, `import Logo from './icon.svg';`); + + const instance = new ReactNativeSVG(tmpDir, tmpDir, false); + instance.setApiTypes(t); + instance.buildSvgMap(); // first call — populates from scan + + const mapAfterFirstCall = { ...instance.localSvgMap }; + + // Simulate what the old code did: create a brand-new instance per file + const freshInstance = new ReactNativeSVG(tmpDir, tmpDir, false); + freshInstance.setApiTypes(t); + freshInstance.buildSvgMap(); // should re-populate identically + + // Both instances should have the same map — the ?? fix ensures the first + // instance is reused rather than a fresh empty one being created + expect(freshInstance.localSvgMap).toEqual(mapAfterFirstCall); + }); +}); diff --git a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/mappers/SvgViewMapper.kt b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/mappers/SvgViewMapper.kt index 7ae53ae2f..c2ec56915 100644 --- a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/mappers/SvgViewMapper.kt +++ b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/mappers/SvgViewMapper.kt @@ -60,20 +60,25 @@ internal open class SvgViewMapper( val wireframes = mutableListOf() if (view is DdPrivacyView) { - val hash = view.attributes?.get("hash") ?: return listOf( - MobileSegment.Wireframe.ShapeWireframe( - resolveViewId(view), - viewGlobalBounds.x, - viewGlobalBounds.y, - viewGlobalBounds.width, - viewGlobalBounds.height, - shapeStyle = shapeStyle, - border = border - ) + // Built once and added up front (before the data guards below) so the view + // still renders as an outlined placeholder if SVG data is unavailable, + // instead of being completely invisible in the replay. + val containerWireframe = MobileSegment.Wireframe.ShapeWireframe( + resolveViewId(view), + viewGlobalBounds.x, + viewGlobalBounds.y, + viewGlobalBounds.width, + viewGlobalBounds.height, + shapeStyle = shapeStyle, + border = border ) + + val hash = view.attributes?.get("hash") ?: return listOf(containerWireframe) val width = view.attributes?.get("width") val height = view.attributes?.get("height") + wireframes.add(containerWireframe) + var entryData = internalCallback.getEntryData(hash) ?: return wireframes @@ -96,16 +101,6 @@ internal open class SvgViewMapper( entryData = entryStr.toByteArray(Charsets.UTF_8); } - wireframes.add(MobileSegment.Wireframe.ShapeWireframe( - resolveViewId(view), - viewGlobalBounds.x, - viewGlobalBounds.y, - viewGlobalBounds.width, - viewGlobalBounds.height, - shapeStyle = shapeStyle, - border = border - )) - val imageWireframeId = viewIdentifierResolver.resolveChildUniqueIdentifier(view, "svg") ?: return wireframes val imgWireframe = MobileSegment.Wireframe.ImageWireframe( imageWireframeId, diff --git a/packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/mappers/SvgViewMapperTest.kt b/packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/mappers/SvgViewMapperTest.kt index 97b01be5e..6d59597f7 100644 --- a/packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/mappers/SvgViewMapperTest.kt +++ b/packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/mappers/SvgViewMapperTest.kt @@ -148,7 +148,9 @@ internal class SvgViewMapperTest { } @Test - fun `M return empty list W map() { DdPrivacyView with hash but no entry data }`() { + fun `M return ShapeWireframe W map() { DdPrivacyView with hash but no entry data }`() { + // A missing assets.bin or unpacked hash should still render the view's + // bounding box rather than making the element invisible in the replay. // Given val hash = "missing-entry-hash" whenever(mockDdPrivacyView.attributes).thenReturn(mapOf("hash" to hash)) @@ -163,11 +165,14 @@ internal class SvgViewMapperTest { ) // Then - assertThat(result).isEmpty() + assertThat(result).hasSize(1) + assertThat(result[0]).isInstanceOf(MobileSegment.Wireframe.ShapeWireframe::class.java) } @Test - fun `M return empty list W map() { DdPrivacyView with hash but no child view }`() { + fun `M return ShapeWireframe W map() { DdPrivacyView with hash but no child view }`() { + // Same fallback: if the Babel-generated child view is missing (e.g. collapsed + // by the Fabric renderer), we still show the container outline. // Given val hash = "no-child-hash" val svgBytes = "".toByteArray(Charsets.UTF_8) @@ -184,6 +189,7 @@ internal class SvgViewMapperTest { ) // Then - assertThat(result).isEmpty() + assertThat(result).hasSize(1) + assertThat(result[0]).isInstanceOf(MobileSegment.Wireframe.ShapeWireframe::class.java) } }