Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update react-native-onyx to reduce setState() calls / renders in withOnyx #4610

Merged
merged 3 commits into from
Aug 13, 2021

Conversation

marcaaron
Copy link
Contributor

@marcaaron marcaaron commented Aug 12, 2021

Details

Context is in the linked issue. This PR implements the changes introduced here.

Fixed Issues

$ #4592 (comment)

Tests

The tests I performed were:

  1. Profile the web app's initial load and inspect the time immediately preceding the FCP
  2. Observed the scripting time
  3. Repeated the test with this branch
  4. Observed a reduction in scripting time

QA Steps

No QA

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@marcaaron marcaaron requested a review from nickmurray47 August 12, 2021 17:24
@marcaaron marcaaron self-assigned this Aug 12, 2021
@marcaaron marcaaron requested a review from a team as a code owner August 12, 2021 17:24
@marcaaron marcaaron removed the request for review from a team August 12, 2021 17:24
@marcaaron marcaaron changed the title Update react-native-onyx to reduce setState() calls / renders in withOnyx [HOLD react-native-onyx] Update react-native-onyx to reduce setState() calls / renders in withOnyx Aug 12, 2021
@MelvinBot MelvinBot requested a review from MariaHCD August 12, 2021 17:25
@marcaaron marcaaron removed the request for review from MariaHCD August 12, 2021 17:25
nickmurray47
nickmurray47 previously approved these changes Aug 13, 2021
@marcaaron marcaaron changed the title [HOLD react-native-onyx] Update react-native-onyx to reduce setState() calls / renders in withOnyx Update react-native-onyx to reduce setState() calls / renders in withOnyx Aug 13, 2021
@marcaaron marcaaron merged commit 496086a into main Aug 13, 2021
@marcaaron marcaaron deleted the marcaaron-updateOnyx branch August 13, 2021 04:21
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@marcaaron marcaaron added the DeployBlockerCash This issue or pull request should block deployment label Aug 16, 2021
@OSBotify
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@github-actions github-actions bot added the Hourly KSv2 label Aug 16, 2021
@marcaaron
Copy link
Contributor Author

Confused, is this on staging or not...? OSBotify says no and it's not on the staging branch - but is referenced on the Deploy Checklist. Oh I see it was added incorrectly maybe... #4554 (comment)

Regardless, we found an issue with this and are fixing it here: #4676

I'm unsure if the new PR should be CP'd to staging. I think not - unless this once moves to staging first and then maybe it should? 🤷

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @marcaaron in version: 1.0.85-10 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@isagoico
Copy link

Removing and re adding the deploy blocker label so the PR is added to the checklist. @marcaaron This PR is now on staging

@isagoico isagoico removed the DeployBlockerCash This issue or pull request should block deployment label Aug 17, 2021
@isagoico isagoico added the DeployBlockerCash This issue or pull request should block deployment label Aug 17, 2021
@OSBotify
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.0.86-11 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DeployBlockerCash This issue or pull request should block deployment Engineering Hourly KSv2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants