Skip to content

Conversation

@alwx
Copy link
Contributor

@alwx alwx commented Sep 19, 2025

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

After we've got rid of react-test-renderer, nothing was blocking us from upgrading core to 0.80. Because some mocks were missing in 0.80.0 (facebook/react-native#51993), the update was done to 0.80.1 straight away.
In addition to that, tests were updated (just because some of them required certain modifications after the RN version bump).

📝 Checklist

  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

#skip-changelog

@alwx alwx self-assigned this Sep 19, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Sep 19, 2025

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 492.06 ms 516.88 ms 24.82 ms
Size 45.86 MiB 50.46 MiB 4.60 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
20d5eaa 377.62 ms 406.50 ms 28.88 ms
c08359e 421.87 ms 445.37 ms 23.50 ms
8490686+dirty 365.59 ms 400.24 ms 34.65 ms
c1573b3 400.85 ms 411.82 ms 10.97 ms
c4e097a 338.06 ms 439.36 ms 101.30 ms
07808fb+dirty 419.10 ms 419.08 ms -0.02 ms
49ef936+dirty 405.96 ms 417.22 ms 11.26 ms
a31630c+dirty 408.91 ms 416.80 ms 7.89 ms
459a438+dirty 417.09 ms 406.52 ms -10.57 ms
05bef0e+dirty 349.78 ms 334.04 ms -15.74 ms

App size

Revision Plain With Sentry Diff
20d5eaa 17.75 MiB 20.15 MiB 2.41 MiB
c08359e 17.75 MiB 20.15 MiB 2.41 MiB
8490686+dirty 17.75 MiB 19.70 MiB 1.96 MiB
c1573b3 17.75 MiB 20.15 MiB 2.41 MiB
c4e097a 17.75 MiB 19.68 MiB 1.94 MiB
07808fb+dirty 17.75 MiB 19.70 MiB 1.95 MiB
49ef936+dirty 17.75 MiB 19.69 MiB 1.94 MiB
a31630c+dirty 17.75 MiB 19.68 MiB 1.94 MiB
459a438+dirty 17.75 MiB 19.70 MiB 1.95 MiB
05bef0e+dirty 17.75 MiB 19.70 MiB 1.95 MiB

Previous results on branch: alwx/improvement/core-0.80

Startup times

Revision Plain With Sentry Diff
d2f50db+dirty 565.81 ms 578.64 ms 12.83 ms

App size

Revision Plain With Sentry Diff
d2f50db+dirty 17.75 MiB 19.68 MiB 1.94 MiB

@github-actions
Copy link
Contributor

github-actions bot commented Sep 19, 2025

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1221.08 ms 1228.48 ms 7.40 ms
Size 2.63 MiB 4.00 MiB 1.36 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
90afdd3+dirty 1233.90 ms 1240.90 ms 7.00 ms
21c9e75+dirty 1237.78 ms 1247.66 ms 9.88 ms
6fee48d+dirty 1222.14 ms 1231.44 ms 9.30 ms
170d5ea+dirty 1219.27 ms 1231.90 ms 12.63 ms
1853710+dirty 1224.35 ms 1230.18 ms 5.84 ms
bc9680d+dirty 1221.41 ms 1241.47 ms 20.06 ms
20d5eaa+dirty 1231.12 ms 1226.00 ms -5.12 ms
276d348+dirty 1224.22 ms 1227.38 ms 3.16 ms
c7f264b+dirty 1211.82 ms 1218.04 ms 6.22 ms
a31630c+dirty 1229.09 ms 1230.94 ms 1.85 ms

App size

Revision Plain With Sentry Diff
90afdd3+dirty 2.63 MiB 3.99 MiB 1.35 MiB
21c9e75+dirty 2.63 MiB 3.81 MiB 1.18 MiB
6fee48d+dirty 2.63 MiB 3.96 MiB 1.33 MiB
170d5ea+dirty 2.63 MiB 3.98 MiB 1.35 MiB
1853710+dirty 2.63 MiB 3.91 MiB 1.28 MiB
bc9680d+dirty 2.63 MiB 3.81 MiB 1.18 MiB
20d5eaa+dirty 2.63 MiB 3.81 MiB 1.18 MiB
276d348+dirty 2.63 MiB 3.98 MiB 1.34 MiB
c7f264b+dirty 2.63 MiB 3.91 MiB 1.28 MiB
a31630c+dirty 2.63 MiB 3.98 MiB 1.34 MiB

Previous results on branch: alwx/improvement/core-0.80

Startup times

Revision Plain With Sentry Diff
d2f50db+dirty 1228.27 ms 1242.14 ms 13.87 ms

App size

Revision Plain With Sentry Diff
d2f50db+dirty 2.63 MiB 3.98 MiB 1.34 MiB

@github-actions
Copy link
Contributor

github-actions bot commented Sep 22, 2025

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 520.66 ms 577.26 ms 56.60 ms
Size 45.86 MiB 50.46 MiB 4.60 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8490686+dirty 344.38 ms 364.37 ms 19.98 ms
20daa0a+dirty 352.33 ms 424.30 ms 71.98 ms
818a608+dirty 350.29 ms 397.38 ms 47.09 ms
69602ce+dirty 375.37 ms 405.28 ms 29.91 ms
b3b5b0d+dirty 361.42 ms 403.90 ms 42.48 ms
07808fb+dirty 392.47 ms 451.94 ms 59.47 ms
49ef936+dirty 333.72 ms 387.51 ms 53.79 ms
a31630c+dirty 359.89 ms 416.90 ms 57.00 ms
459a438+dirty 359.50 ms 390.53 ms 31.03 ms
05bef0e+dirty 451.63 ms 533.90 ms 82.27 ms

App size

Revision Plain With Sentry Diff
8490686+dirty 7.15 MiB 8.43 MiB 1.28 MiB
20daa0a+dirty 7.15 MiB 8.42 MiB 1.27 MiB
818a608+dirty 7.15 MiB 8.41 MiB 1.26 MiB
69602ce+dirty 7.15 MiB 8.41 MiB 1.26 MiB
b3b5b0d+dirty 7.15 MiB 8.41 MiB 1.26 MiB
07808fb+dirty 7.15 MiB 8.43 MiB 1.28 MiB
49ef936+dirty 7.15 MiB 8.42 MiB 1.26 MiB
a31630c+dirty 7.15 MiB 8.41 MiB 1.26 MiB
459a438+dirty 7.15 MiB 8.42 MiB 1.27 MiB
05bef0e+dirty 7.15 MiB 8.43 MiB 1.28 MiB

Previous results on branch: alwx/improvement/core-0.80

Startup times

Revision Plain With Sentry Diff
d2f50db+dirty 334.22 ms 358.23 ms 24.00 ms

App size

Revision Plain With Sentry Diff
d2f50db+dirty 7.15 MiB 8.41 MiB 1.26 MiB

@alwx alwx changed the title Bump core to RN 0.80 Bump core to RN 0.80.1 Sep 25, 2025
@alwx alwx marked this pull request as ready for review September 25, 2025 11:42
@antonis
Copy link
Contributor

antonis commented Sep 29, 2025

Note: I've took the liberty of adding the #skip-changelog to make the CI happy. Checking the latest bumps (#4560, #4331, #4173) since we usually don't communicate this kind of internal change.

],
"peerDependencies": {
"expo": ">=49.0.0",
"react": ">=17.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

q: Why do we remove the "react" peer depencency?

includeUpdates: true,
}),
expect.anything(),
expect.toBeNil()
Copy link
Contributor

Choose a reason for hiding this comment

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

q: Do we know why the second parameter changed? Does it affect the profiler?

Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Thank you for you work on this @alwx 🙇
Overall the changes LGTM 🚀

I think the only blocking thing is the Native test failure on iOS. I haven't looked at it in detail but some of the errors might be related to the folly. We added some check here some time ago. Probably the Cocoa Tester needs something similar 🤔

@alwx alwx force-pushed the alwx/improvement/core-0.80 branch 2 times, most recently from 33bf13b to dd7d431 Compare September 30, 2025 08:59
@github-actions
Copy link
Contributor

github-actions bot commented Sep 30, 2025

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1236.81 ms 1235.50 ms -1.31 ms
Size 3.19 MiB 4.54 MiB 1.36 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
bc9680d+dirty 1228.57 ms 1233.64 ms 5.07 ms
c9e95bd+dirty 1205.83 ms 1207.38 ms 1.55 ms
c1573b3+dirty 1245.69 ms 1245.48 ms -0.21 ms
49ef936+dirty 1221.27 ms 1221.60 ms 0.34 ms
8d89cc9+dirty 1232.35 ms 1228.53 ms -3.82 ms
e2fa43d+dirty 1224.43 ms 1235.40 ms 10.98 ms
9f211e3+dirty 1215.38 ms 1218.15 ms 2.77 ms
7be1f99+dirty 1222.43 ms 1217.15 ms -5.28 ms
21c9e75+dirty 1206.20 ms 1223.54 ms 17.35 ms
5c16cdc+dirty 1235.67 ms 1241.18 ms 5.51 ms

App size

Revision Plain With Sentry Diff
bc9680d+dirty 3.19 MiB 4.38 MiB 1.19 MiB
c9e95bd+dirty 3.19 MiB 4.44 MiB 1.25 MiB
c1573b3+dirty 3.19 MiB 4.38 MiB 1.19 MiB
49ef936+dirty 3.19 MiB 4.54 MiB 1.36 MiB
8d89cc9+dirty 3.19 MiB 4.53 MiB 1.35 MiB
e2fa43d+dirty 3.19 MiB 4.38 MiB 1.19 MiB
9f211e3+dirty 3.19 MiB 4.48 MiB 1.29 MiB
7be1f99+dirty 3.19 MiB 4.38 MiB 1.19 MiB
21c9e75+dirty 3.19 MiB 4.38 MiB 1.19 MiB
5c16cdc+dirty 3.19 MiB 4.53 MiB 1.34 MiB

@alwx alwx force-pushed the alwx/improvement/core-0.80 branch 2 times, most recently from f293655 to 5f9c0aa Compare October 6, 2025 13:53
@lucas-zimerman
Copy link
Collaborator

@sentry review

Comment on lines 13 to +33
};
const mockedAppState: AppState & MockAppState = {
removeSubscription: jest.fn(),
listener: jest.fn(),
isAvailable: true,
currentState: 'active',
addEventListener: (_, listener) => {
mockedAppState.listener = listener;
return {
remove: mockedAppState.removeSubscription,
};
},
setState: (state: AppStateStatus) => {
mockedAppState.currentState = state;
mockedAppState.listener(state);
},
};
jest.mock('react-native/Libraries/AppState/AppState', () => mockedAppState);
jest.mock('react-native', () => {
const mockedAppState: AppState & MockAppState = {
removeSubscription: jest.fn(),
listener: jest.fn(),
isAvailable: true,
currentState: 'active',
addEventListener: jest.fn(),
setState: (state: AppStateStatus) => {
mockedAppState.currentState = state;
mockedAppState.listener(state);
},
};
return {
AppState: mockedAppState,
Platform: { OS: 'ios' },
NativeModules: {
RNSentry: {},
},
};

Choose a reason for hiding this comment

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

The mock setup creates a circular reference where mockedAppState.addEventListener and mockedAppState.setState reference mockedAppState before it's fully initialized. While this works in JavaScript due to hoisting, it's a code smell. Consider initializing addEventListener and setState after the object is created, or restructuring the mock to avoid this pattern.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +30 to +49
jest.mock('react-native', () => {
const mockedAppState: AppState & MockAppState = {
removeSubscription: jest.fn(),
listener: jest.fn(),
isAvailable: true,
currentState: 'active',
addEventListener: jest.fn(),
setState: (state: AppStateStatus) => {
mockedAppState.currentState = state;
mockedAppState.listener(state);
},
};
return {
AppState: mockedAppState,
Platform: { OS: 'ios' },
NativeModules: {
RNSentry: {},
},
};
});

Choose a reason for hiding this comment

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

Similar to the idleNavigationSpan test, the mock setup creates a circular reference in the object initialization. Consider restructuring the mock to avoid this pattern for better code clarity and maintainability.

Did we get this right? 👍 / 👎 to inform future reviews.

showFeedbackButton();

fireEvent.press(getByText('Report a Bug'));
await waitFor(() => fireEvent.press(getByText('Report a Bug')));

Choose a reason for hiding this comment

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

On line 88, the pattern await waitFor(() => fireEvent.press(...)) is unusual. Typically, fireEvent.press() is synchronous and doesn't need to be wrapped in waitFor().
The waitFor() is usually used for waiting for asynchronous side effects after the event. Consider whether this should be fireEvent.press(...) followed by await waitFor(() => expect(...)) for better clarity.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +109 to +112
await waitFor(() => {
fireEvent.press(getByText('Report a Bug'));
fireEvent.press(getByText('Take a screenshot'));
});

Choose a reason for hiding this comment

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

Multiple instances of wrapping synchronous fireEvent.press() calls inside waitFor() (lines 109-112, 135-138, 167-170, 213-216) suggest a misunderstanding of the testing pattern. fireEvent operations are synchronous; only the assertions of their effects typically need waitFor(). This could lead to unnecessary delays in test execution.

Did we get this right? 👍 / 👎 to inform future reviews.

includeUpdates: true,
}),
expect.anything(),
expect.toBeNil()

Choose a reason for hiding this comment

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

The change from expect.anything() to expect.toBeNil() appears to be checking for a nil/null value. However, toBeNil() is not a standard Jest matcher. This might be a custom matcher from jest-extended, but it should be verified that this change correctly validates the expected behavior. Standard Jest would use expect(value).toBeNull() or expect(value).toBeUndefined().

Did we get this right? 👍 / 👎 to inform future reviews.

updateProps: {},
}),
expect.anything(),
expect.toBeNil(),

Choose a reason for hiding this comment

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

Similar to the previous comment, verify that expect.toBeNil() is the correct matcher and is properly imported/configured. If this is from jest-extended, ensure it's properly set up in the test configuration.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines 72 to 73
const versionNumber = parseFloat(rnVersion.replace(/[^\d.]/g, ''));
const shouldRunCodegen = platform === 'android' && versionNumber >= 0.80;
const shouldRunCodegen = platform === 'android' && versionNumber >= 0.69;

Choose a reason for hiding this comment

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

The version threshold change from 0.80 to 0.69 for running codegen needs to be carefully verified. According to the repository context, Metro version pinning is applied only for React Native versions 0.80 and above. This change to 0.69 might require corresponding updates to the Metro version pinning logic in rn.patch.package.json.js to ensure consistency. Verify that RN 0.69 doesn't have the same Metro issues mentioned in the context.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines 107 to 115
const versionNumber = parseFloat(RNVersion.replace(/[^\d.]/g, ''));
if (versionNumber >= 0.80) {
if (versionNumber >= 0.69) {
execSync(`${patchScriptsDir}/rn.patch.package.json.js --path package.json`, {
stdio: 'inherit',
cwd: appDir,
env: env,
});
} else {
console.log(`Skipping rn.patch.package.json.js for RN ${RNVersion} (< 0.80)`);
console.log(`Skipping rn.patch.package.json.js for RN ${RNVersion} (< 0.69)`);

Choose a reason for hiding this comment

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

The version threshold for applying rn.patch.package.json.js has been lowered from 0.80 to 0.69. According to the repository-specific context, Metro version pinning via this patch script is critical for RN 0.80+ to prevent E2E test failures. Ensure that applying this patch to RN 0.69 is safe and doesn't introduce unintended side effects, or update the patch script logic to handle different RN versions appropriately.

Did we get this right? 👍 / 👎 to inform future reviews.

"prettier": "^2.0.5",
"react": "18.3.1",
"react-native": "0.77.1",
"react": "19.1.0",

Choose a reason for hiding this comment

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

The update from React 18.3.1 to 19.1.0 is a major version upgrade that introduces breaking changes. Ensure that all React 19 breaking changes have been reviewed and that the codebase is compatible. Specifically, React 19 has changes to refs, context, and hooks behavior that may require code updates beyond just dependency version bumps.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines 652 to 653
animationType="none"
hardwareAccelerated={false}
onRequestClose={[Function]}

Choose a reason for hiding this comment

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

The removal of the hardwareAccelerated={false} prop from Modal snapshots indicates a change in React Native's Modal component API between versions. While snapshot updates are expected with dependency upgrades, verify that this prop removal doesn't affect the actual runtime behavior of modals in the application, especially on Android where hardware acceleration can impact performance.

Did we get this right? 👍 / 👎 to inform future reviews.

@alwx alwx force-pushed the alwx/improvement/core-0.80 branch 4 times, most recently from f5710f4 to c695a04 Compare October 16, 2025 11:59
@alwx alwx force-pushed the alwx/improvement/core-0.80 branch 4 times, most recently from 73bb9e1 to 4a57918 Compare October 20, 2025 12:56
Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

The changes LGTM 🚀
Thank you for all your work on this @alwx 🙇

@alwx alwx force-pushed the alwx/improvement/core-0.80 branch 4 times, most recently from f8c3f6b to 32d6cd0 Compare October 22, 2025 12:11
@lucas-zimerman
Copy link
Collaborator

Looking good! the changed kotlin files from the test app should be linted, after that we could merge this PR!

@alwx alwx force-pushed the alwx/improvement/core-0.80 branch from 95e360d to b8689f4 Compare October 23, 2025 14:42
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