[NOJIRA] [FIX] [V2] Remove flexShrink:1 from SVG transformation on Babel Plugin#1275
Conversation
There was a problem hiding this comment.
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.
| function transformWithSvgTracking(code: string): string | undefined { | ||
| const tmpDir = path.join(os.tmpdir(), 'dd-svg-test-assets'); | ||
| const reactNativeSVG = new ReactNativeSVG(process.cwd(), tmpDir); | ||
|
|
There was a problem hiding this comment.
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.
| 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; |
There was a problem hiding this comment.
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.
| 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'); | ||
| }); |
There was a problem hiding this comment.
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.
cdn34dd
force-pushed
the
carlosnogueira/fix/babel-plugin-svg-tracking-flex-shrink-issue-v2
branch
from
036afc6 to
0246730
Compare
cdn34dd
deleted the
carlosnogueira/fix/babel-plugin-svg-tracking-flex-shrink-issue-v2
branch
What does this PR do?
Removes the
flexShrink: 1that was being set as part of the styles to theSessionReplayView.Privacywrapper view that is set by Babel Plugin around SVG elements so they are trackable by SR. This was causing layout issues, as reported by #1193This 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)