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.

Sign up for GitHub

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

Jump to bottom

Swaps performance improvements, logic fixes #6050

Merged
merged 24 commits into from
Aug 30, 2024
Merged

Swaps performance improvements, logic fixes #6050

merged 24 commits into from
Aug 30, 2024

Conversation

christianbaroni
Copy link
Member

@christianbaroni christianbaroni commented Aug 27, 2024 •
edited
Loading

Fixes APP-1766

What changed (plus any additional context for devs)

  • Improves performance throughout swaps (see commit history for details)
  • Main improvements:
    • Optimizes useAnimatedReaction updates — previously there were lots of unnecessary updates due to no dependency array being passed (now passing an empty array in most cases, which limits reactions to changes in the prepare function values)
    • Migrates the token lists from FlashList to FlatList, which seems overall more performant
    • Cleans up the swap confirm button logic to reduce jank

Screen recordings / screenshots

What to test

@christianbaroni
Clean up swap coin icons, fix flashes
9d6fc28
@christianbaroni
Fix useAnimatedReaction dependencies in swaps
1e47176 
Without a dependency array, useAnimatedReaction reacts to changes in any value used throughout the useAnimatedReaction, whereas an empty dependency array limits reactions to changes in values used in the prepare function.
@christianbaroni
Show disabled paste button when clipboard is empty
156dc6d 
Previously it was fully hidden
@christianbaroni
Make hasClipboardData react to clipboard changes
e708ef0
@christianbaroni
Remove redundant Flashbots disabling
5ae9e14 
The row containing the switch already gets hidden when Flashbots is disabled
@christianbaroni
Expose runOnUIImmediately
cdc7b0f 
This is an alternative to runOnUI that allows bypassing reanimated's internal update batching and the accompanying possibility of a delay before code gets executed on the UI thread
@christianbaroni
Remove redundant resetting of hasEnoughFundsForGas
50cbbf3 
It's already set to undefined any time the gas fee is recalculated in SyncGasStateToSharedValues
@christianbaroni
Clean up swap confirm button logic to reduce jank
150cfe4
@christianbaroni
Stop auto-fetching quotes for certain error cases
3d68519
@christianbaroni
Use runOnUIImmediately for time-sensitive interactions
436e792
@christianbaroni
Reduce hold to swap time from 500ms to 400ms
5b16cb3
@christianbaroni
Fix max button edge cases
1d4037d
@christianbaroni
Migrate token lists from FlashList to FlatList
8257b00 
FlatList seems to be more performant overall, and far fewer instances of blank list cells
@christianbaroni
Reduce jank when the input asset's chain changes
4db9dee
@christianbaroni
Improve the inputValues useAnimatedReaction equality check
7d8b52a 
Some unnecessary updates were getting through with the current !== previous check
@christianbaroni
Fix outputAmount zero value handling
bd982fe
@christianbaroni
Add missing hold_to_bridge i18n string
7b42b0d
@christianbaroni
Move gas calculations to the UI thread
7581858
@christianbaroni
Add missing export
25cf20c
@christianbaroni
Fix userAssets RegExp bug
f97e138 
Certain escape characters like "\" previously triggered a crash if entered into the input asset search
@linear Linear
Copy link

linear bot commented Aug 28, 2024

APP-1766 Paste button should always be shown in Swaps UI

@christianbaroni
Revert "Expose runOnUIImmediately"
a585e7a 
This reverts commit cdc7b0f.
@christianbaroni
Revert "Use runOnUIImmediately for time-sensitive interactions"
4c93906 
This reverts commit 436e792.

# Conflicts:
#	src/__swaps__/screens/Swap/components/TokenList/TokenToBuyList.tsx
@brunobar79 brunobar79 requested review from benisgold and removed request for greg-schrammel August 28, 2024 17:20
@jinchung jinchung removed their request for review August 28, 2024 17:21
walmat
walmat approved these changes Aug 30, 2024
View reviewed changes
Copy link
Contributor

@walmat walmat left a comment

Choose a reason for hiding this comment

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

tested android and ios no regressions and seems to be the same or maybe slightly faster on both. Good work! Couple questions mostly for clarity.

@christianbaroni christianbaroni merged commit 12abd5e into develop Aug 30, 2024
5 checks passed
@christianbaroni christianbaroni deleted the @christian/swaps-improvements branch August 30, 2024 18:30
@sentry-io Sentry.io
Copy link

sentry-io bot commented Sep 8, 2024 •
edited
Loading

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ React ErrorBoundary TypeError: Cannot convert undefined value to object anonymous(src/__swaps__/screens/Swap/hooks/useS... View Issue
  • ‼️ React ErrorBoundary TypeError: Cannot convert undefined value to object anonymous(src/__swaps__/screens/Swap/hooks/useS... View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greg-schrammel greg-schrammel greg-schrammel left review comments

@walmat walmat walmat approved these changes

@benisgold benisgold Awaiting requested review from benisgold

Assignees
No one assigned
Labels
swaps v2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@christianbaroni @greg-schrammel @walmat