Skip to content

[NOJIRA] [FIX] [V2] Remove flexShrink:1 from SVG transformation on Babel Plugin#1275

Open
cdn34dd wants to merge 1 commit intofeature/v2from
carlosnogueira/fix/babel-plugin-svg-tracking-flex-shrink-issue-v2
Open

[NOJIRA] [FIX] [V2] Remove flexShrink:1 from SVG transformation on Babel Plugin#1275
cdn34dd wants to merge 1 commit intofeature/v2from
carlosnogueira/fix/babel-plugin-svg-tracking-flex-shrink-issue-v2

Conversation

@cdn34dd
Copy link
Copy Markdown
Contributor

@cdn34dd cdn34dd commented Apr 27, 2026

What does this PR do?

Removes the flexShrink: 1 that was being set as part of the styles to the SessionReplayView.Privacy wrapper view that is set by Babel Plugin around SVG elements so they are trackable by SR. This was causing layout issues, as reported by #1193

This caused the wrapped icons to be rendered in smaller size instead of keeping them intact. This is due to React Native default to flexShrink: 0 instead of 1, as normal CSS does on web.

Motivation

Session Replay should not modify the host app's UI in any way.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)
  • If this PR is auto-generated, please make sure also to manually update the code related to the change

Copilot AI review requested due to automatic review settings April 27, 2026 17:54
@cdn34dd cdn34dd requested a review from a team as a code owner April 27, 2026 17:54
Copy link
Copy Markdown

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

Removes the flexShrink: 1 style that the Babel plugin was adding to the SessionReplayView.Privacy wrapper around react-native-svg elements, preventing Session Replay SVG tracking from altering host-app layout (issue #1193).

Changes:

  • Remove the injected style={{ flexShrink: 1 }} from the Session Replay SVG wrapper in the React Native SVG processing logic.
  • Add Babel-plugin-level tests ensuring SVG wrapping behavior and asserting no wrapper style is added.
  • Add Jest snapshots for the new wrapper behavior tests.

Reviewed changes

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

File Description
packages/react-native-babel-plugin/test/react-native-svg.test.ts Adds end-to-end Babel plugin tests for SVG wrapping and asserts the wrapper does not add style/flexShrink.
packages/react-native-babel-plugin/test/snapshots/react-native-svg.test.ts.snap Adds snapshots capturing the transformed wrapper output.
packages/react-native-babel-plugin/src/libraries/react-native-svg/index.ts Removes generation of the style prop (with flexShrink) from SessionReplayView.Privacy wrapper attributes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +774 to +777
function transformWithSvgTracking(code: string): string | undefined {
const tmpDir = path.join(os.tmpdir(), 'dd-svg-test-assets');
const reactNativeSVG = new ReactNativeSVG(process.cwd(), tmpDir);

Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

The test uses a fixed temp directory name (os.tmpdir()/dd-svg-test-assets). When Jest runs in parallel workers, multiple suites/processes can write/remove the same directory, causing racy failures and non-deterministic snapshots. Use a unique temp dir per test file run (e.g., fs.mkdtempSync(path.join(os.tmpdir(), 'dd-svg-test-assets-')) or include process.pid/JEST_WORKER_ID), and pass that directory into transformWithSvgTracking and the cleanup hook.

Copilot uses AI. Check for mistakes.
Comment on lines +778 to +791
return transform(code, {
filename: 'file.tsx',
presets: ['@babel/preset-react', '@babel/preset-typescript'],
plugins: [
[
plugin,
{
sessionReplay: { svgTracking: true },
__internal_reactNativeSVG: reactNativeSVG
}
]
],
configFile: false
})?.code as string | undefined;
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

insertSetupFlag only runs once per PluginState platform, so the first transformWithSvgTracking call will include the globalThis.__DD_RN_BABEL_PLUGIN_ENABLED__ assignment and subsequent calls won’t. This makes the snapshot outputs/order-dependent within this file. Consider warming up the plugin once (run a dummy transform before snapshotting), or normalize the transformed code for snapshots (e.g., strip the setup-flag statement), so snapshots are stable regardless of test ordering.

Copilot uses AI. Check for mistakes.
Comment on lines +821 to +828
it('should not set a style prop on the wrapper', () => {
const input =
'<Svg width="24" height="24"><Path d="M12 2L2 12h10z" fill="black" /></Svg>';
const output = transformWithSvgTracking(input);

expect(output).not.toContain('flexShrink');
expect(output).not.toContain('style');
});
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

expect(output).not.toContain('style') is overly broad: it will fail if the transformed code contains any style identifier anywhere (including on the original <Svg> or other nodes), even though the intent is only to ensure the wrapper doesn’t add a style prop. Prefer asserting specifically on the wrapper props (e.g., match the SessionReplayView.Privacy createElement/object literal and ensure it lacks style) or parse the output AST to check for a style prop on that element only.

Copilot uses AI. Check for mistakes.
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