Skip to content

Commit 4f31770

Browse files
committed
Prevent unresolvable SVG attributes from dropping the entire generated asset
1 parent 6f38a14 commit 4f31770

3 files changed

Lines changed: 156 additions & 14 deletions

File tree

packages/react-native-babel-plugin/src/libraries/react-native-svg/handlers/RNSvgHandler.ts

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,17 @@ export class RNSvgHandler implements SvgHandler {
9292
* @param t - Babel types helper.
9393
* @param el - JSXElement node to transform.
9494
* @param dimensions - Optional object to collect extracted width/height info.
95+
* @returns `true` if the element is supported and was transformed in place. `false` if
96+
* the element is not a recognized SVG tag (e.g. a custom wrapper component like
97+
* `Animated.createAnimatedComponent(Path)`) and must be removed by the caller — see
98+
* `handleRegularAttributes` for why leaving it in place isn't an option.
9599
*/
96100
private transformElement(
97101
t: typeof Babel.types,
98102
rootElementPath: Babel.NodePath<Babel.types.JSXElement> | null,
99103
el: Babel.types.JSXElement,
100104
dimensions: Record<string, string>
101-
) {
105+
): boolean {
102106
const openingNode = el.openingElement.name;
103107
const isJSXIdentifierOpen = t.isJSXIdentifier(openingNode);
104108

@@ -107,9 +111,9 @@ export class RNSvgHandler implements SvgHandler {
107111
openingNode.name = convertAttributeCasing(openingNode.name);
108112
if (!svgElements.has(openingNode.name)) {
109113
console.warn(
110-
`RNSvgHandler[transformElement]: Skipping unsupported element: "${openingNode.name}"`
114+
`RNSvgHandler[transformElement]: Removing unsupported element: "${openingNode.name}"`
111115
);
112-
return; // Skip unsupported elements instead of crashing
116+
return false; // Signal the caller to remove this element from the tree
113117
}
114118
}
115119

@@ -128,11 +132,12 @@ export class RNSvgHandler implements SvgHandler {
128132
}
129133

130134
this.processAttributes(t, rootElementPath, el, dimensions);
135+
return true;
131136
}
132137

133138
/**
134139
* Recursively traverses the children of a JSXElement and applies `transformElement`
135-
* to each child that is itself a JSXElement.
140+
* to each child that is itself a JSXElement, splicing out any child reported unsupported.
136141
*
137142
* @param t - Babel types helper.
138143
* @param rootElementPath - The path of the root JSX element containing the SVG.
@@ -147,9 +152,21 @@ export class RNSvgHandler implements SvgHandler {
147152
jsxElement: Babel.types.JSXElement,
148153
dimensions: Record<string, string> = {}
149154
) {
150-
for (const child of jsxElement.children) {
155+
const children = jsxElement.children;
156+
157+
// Iterate in reverse so splicing doesn't shift the indices of unvisited entries.
158+
for (let i = children.length - 1; i >= 0; i--) {
159+
const child = children[i];
151160
if (t.isJSXElement(child)) {
152-
this.transformElement(t, rootElementPath, child, dimensions);
161+
const isSupported = this.transformElement(
162+
t,
163+
rootElementPath,
164+
child,
165+
dimensions
166+
);
167+
if (!isSupported) {
168+
children.splice(i, 1);
169+
}
153170
}
154171
}
155172
}
@@ -281,7 +298,9 @@ export class RNSvgHandler implements SvgHandler {
281298
continue;
282299
}
283300

284-
handleRegularAttributes(t, attr);
301+
if (handleRegularAttributes(t, attr)) {
302+
el.attributes.splice(index, 1);
303+
}
285304
} catch (error) {
286305
console.error('ReactNativeSVG[processAttributes]: ', error);
287306
}

packages/react-native-babel-plugin/src/libraries/react-native-svg/processing/attributes.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -325,20 +325,31 @@ export function handleJoinedTransformAttributes(
325325

326326
/**
327327
* Converts a standard JSX attribute (e.g., `stroke`, `fill`, `opacity`) to a string literal
328-
* if it holds a valid value.
328+
* if it holds a statically resolvable value.
329329
*
330330
* @param t - Babel types helper.
331331
* @param attr - JSX attribute node.
332+
* @returns `true` if the attribute should be removed from the element — i.e. its value is a
333+
* JSX expression that could not be statically resolved and would produce invalid SVG XML
334+
* (e.g. `fill={color}` where `color` is a prop or variable). `false` if the attribute was
335+
* successfully converted or required no change.
332336
*/
333337
export function handleRegularAttributes(
334338
t: typeof Babel.types,
335339
attr: Babel.types.JSXAttribute
336-
) {
340+
): boolean {
337341
const result = getJSXAttributeData(t, attr);
338342

339-
if (result.value) {
343+
// Use !== null rather than truthiness: 0, '', and false are all valid resolved
344+
// values (e.g. opacity={0}) and must not be treated as unresolved.
345+
if (result.value !== null) {
340346
attr.value = t.stringLiteral(result.value.toString());
347+
return false;
341348
}
349+
350+
// Unresolved JSX expression containers (e.g. fill={color}) would otherwise serialize
351+
// into invalid SVG XML and make svgo throw — remove them instead.
352+
return t.isJSXExpressionContainer(attr.value);
342353
}
343354

344355
/**

packages/react-native-babel-plugin/test/react-native-svg.test.ts

Lines changed: 116 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -735,23 +735,71 @@ describe('React Native SVG Processing - RNSvgHandler', () => {
735735
});
736736

737737
describe('Error Handling', () => {
738-
it('should warn for unsupported element names but still include them in output', () => {
738+
it('should warn for unsupported element names and remove them from output', () => {
739739
const warnSpy = jest.spyOn(console, 'warn').mockImplementation();
740740

741741
const input = '<Svg><UnsupportedElement x="10" y="10" /></Svg>';
742742
const output = transformSvg(input);
743743

744-
// Unsupported elements are converted to lowercase and included
744+
// Unsupported elements are now removed entirely rather than left in place.
745745
expect(output).toMatchInlineSnapshot(
746-
`"<svg xmlns="http://www.w3.org/2000/svg"><unsupportedElement x="10" y="10" /></svg>"`
746+
`"<svg xmlns="http://www.w3.org/2000/svg"></svg>"`
747747
);
748748
expect(warnSpy).toHaveBeenCalledWith(
749-
expect.stringContaining('Skipping unsupported element')
749+
expect.stringContaining('Removing unsupported element')
750750
);
751751

752752
warnSpy.mockRestore();
753753
});
754754

755+
it('should remove an unsupported element but keep its supported siblings', () => {
756+
const input =
757+
'<Svg><Circle cx="50" cy="50" r="40" fill="#27ae60" /><UnsupportedElement x="10" y="10" /></Svg>';
758+
const output = transformSvg(input);
759+
760+
expect(output).toContain(
761+
'<circle cx="50" cy="50" r="40" fill="#27ae60" />'
762+
);
763+
expect(output).not.toContain('unsupportedElement');
764+
});
765+
766+
it('should remove an unsupported element with dynamic/unresolvable attributes without throwing', () => {
767+
// Mirrors the customer repro: a custom component (e.g. AnimatedPath from
768+
// Animated.createAnimatedComponent) with JSX expression container attributes
769+
// that cannot be statically resolved (fill={color}, style={{opacity}}).
770+
const input = `
771+
function Icon({ color, opacity }) {
772+
return (
773+
<Svg width="64" height="64" viewBox="0 0 64 64">
774+
<Circle cx="32" cy="32" r="28" fill="#27ae60" />
775+
<AnimatedPath
776+
d="M18 32 L28 42 L46 20"
777+
fill={color}
778+
style={{ opacity }}
779+
/>
780+
</Svg>
781+
);
782+
}
783+
`;
784+
785+
expect(() => transformSvg(input)).not.toThrow();
786+
787+
const output = transformSvg(input);
788+
expect(output).toContain(
789+
'<circle cx="32" cy="32" r="28" fill="#27ae60" />'
790+
);
791+
expect(output).not.toContain('animatedPath');
792+
expect(output).not.toContain('fill={color}');
793+
});
794+
795+
it('should preserve resolvable falsy values (0) instead of treating them as unresolved', () => {
796+
const input =
797+
'<Svg><Circle cx="50" cy="50" r="40" opacity={0} /></Svg>';
798+
const output = transformSvg(input);
799+
800+
expect(output).toContain('opacity="0"');
801+
});
802+
755803
it('should handle malformed transform array gracefully', () => {
756804
const input =
757805
'<Svg><Rect x="10" y="10" width="50" height="50" transform={[null, undefined, { invalid: true }]} /></Svg>';
@@ -826,4 +874,68 @@ describe('SessionReplayView.Privacy SVG Wrapper', () => {
826874
expect(output).not.toContain('flexShrink');
827875
expect(output).not.toContain('style');
828876
});
877+
878+
it('should capture an SVG whose child element has a non-resolvable prop, dropping only that attr from the asset', () => {
879+
const assetDir = path.join(os.tmpdir(), 'dd-svg-test-assets');
880+
const input = `
881+
function Icon({ color }) {
882+
return (
883+
<Svg width="80" height="80" viewBox="0 0 100 100">
884+
<Circle cx={50} cy={50} r={40} fill={color} />
885+
</Svg>
886+
);
887+
}
888+
`;
889+
const output = transformWithSvgTracking(input);
890+
891+
// The wrapper must be present — the element was NOT silently dropped
892+
expect(output).toContain('SessionReplayView.Privacy');
893+
// The wrapper carries a hash attribute — the SVG passed through svgo without error
894+
expect(output).toContain('hash:');
895+
896+
// The written SVG asset must not contain the unresolvable expression.
897+
// (Reading the file proves svgo successfully processed the content.)
898+
const svgFiles = fs
899+
.readdirSync(assetDir)
900+
.filter(f => f.endsWith('.svg'));
901+
expect(svgFiles.length).toBeGreaterThan(0);
902+
const svgContent = fs.readFileSync(
903+
path.join(assetDir, svgFiles[svgFiles.length - 1]),
904+
'utf8'
905+
);
906+
expect(svgContent).not.toContain('fill={color}');
907+
// Numeric literals on child elements are resolved and kept
908+
expect(svgContent).toContain('cx="50"');
909+
expect(svgContent).toContain('cy="50"');
910+
expect(svgContent).toContain('r="40"');
911+
});
912+
913+
it('should capture an SVG where all child props are non-resolvable, producing a shape-only SVG', () => {
914+
const assetDir = path.join(os.tmpdir(), 'dd-svg-test-assets');
915+
const input = `
916+
function Icon({ d, fill, strokeWidth }) {
917+
return (
918+
<Svg width="24" height="24" viewBox="0 0 24 24">
919+
<Path d={d} fill={fill} strokeWidth={strokeWidth} />
920+
</Svg>
921+
);
922+
}
923+
`;
924+
const output = transformWithSvgTracking(input);
925+
926+
// Still wrapped — the SVG container is captured even if all child attrs are dynamic
927+
expect(output).toContain('SessionReplayView.Privacy');
928+
929+
const svgFiles = fs
930+
.readdirSync(assetDir)
931+
.filter(f => f.endsWith('.svg'));
932+
expect(svgFiles.length).toBeGreaterThan(0);
933+
const svgContent = fs.readFileSync(
934+
path.join(assetDir, svgFiles[svgFiles.length - 1]),
935+
'utf8'
936+
);
937+
expect(svgContent).not.toContain('fill={fill}');
938+
expect(svgContent).not.toContain('d={d}');
939+
expect(svgContent).not.toContain('strokeWidth={strokeWidth}');
940+
});
829941
});

0 commit comments

Comments
 (0)