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

Browser: new architecture #5671

Merged
merged 39 commits into from
May 1, 2024
Merged

Conversation

christianbaroni
Copy link
Member

@christianbaroni christianbaroni commented Apr 26, 2024

What changed (plus any additional context for devs)

  • Introduces a new browser architecture that across the browser, restricts re-renders to the absolute minimum number required
    • Eliminates all unneeded re-renders within the Homepage component
    • There are now zero re-renders in DappBrowser
    • The number of re-renders that occur across the browser is now exactly the same whether you have 2 tabs or 50 tabs (except for a few things on the homepage, but the impact is minimized with React.memo)
      • No re-renders occur when navigating within the WebView itself
      • The only time a tab re-renders is if either its internal active state changes, or if it's forced to load a new page via goToUrl
  • Fixes all URL/single-page app issues via animatedTabUrls, which serves as the always-up-to-date source of truth for tab URLs
    • This allowed ditching the state responsible for the previous re-renders on navigation
    • This also fixed all remaining screenshot bugs caused by not always having the correct page URL
  • Leverages the new store and lightweight persistence introduced in Browser: browserStore #5669 to create the BrowserTab instances in the most lightweight way possible, mapping each tabId in the tabIds array onto a BrowserTab instance
  • Leverages useSyncSharedValue from useSyncSharedValue #5662 to replace the previous queueing system, which effectively guarantees proper state syncing between the UI ↔ JS threads and fixes all broken tab layout issues resulting from out-of-sync states
    • Adds a new pauseSync feature, and improves the equality check worklets used within
  • Reduces duplication of animated styles and values where possible - now sharing multipleTabsOpen, animatedWebViewHeight, and a few other things
  • Splits expensive animated styles and inexpensive ones, which smooths out the tab animations
  • Lots of other smaller misc. optimizations and fixes

TODO:

  • Get Dapp Browser: Search Performance #5645 integrated
  • Reintegrate canGoBack / canGoForward (navigation works but both buttons are always visible)
  • Finish integrating favorites into the new URL system
  • Hook up the ControlPanel to reflect the current site and allow for wallet and network switching
  • Refactor the old AccountIcon and ensure the appSessions store works within the new system
  • Re-enable screenshot pruning - needs some small adjustments to work within the new architecture
  • Catch up with develop
    • Reapply Skylar's Android fixes

Screen recordings / screenshots

Demo with a hardcoded new tab URL — the browser no longer slows down for every new tab, because the number of re-renders doesn't multiply:

RPReplay_Final1713857597.MP4

What to test

@christianbaroni christianbaroni marked this pull request as draft April 26, 2024 05:01
Copy link
Member

@brunobar79 brunobar79 left a comment

Choose a reason for hiding this comment

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

Good stuff!

const backgroundColor = useSharedValue<string>(defaultBackgroundColor);

const safeBackgroundColor = useDerivedValue(() => {
const homepageColor = isDarkMode ? globalColors.grey100 : '#FBFCFD';
Copy link
Member

Choose a reason for hiding this comment

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

This one is used in a bunch of places inside the dapp browser. Should we add it to the colors?

}, [animatedTabIndex]);

const animatedTabYPosition = useDerivedValue(() => {
return withTiming(Math.floor(animatedTabIndex.value / 2) * TAB_VIEW_ROW_HEIGHT - 181, TIMING_CONFIGS.tabPressConfig);
Copy link
Member

Choose a reason for hiding this comment

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

magic number

src/components/DappBrowser/BrowserTab.tsx Outdated Show resolved Hide resolved
src/components/DappBrowser/BrowserTab.tsx Outdated Show resolved Hide resolved
src/components/DappBrowser/BrowserTab.tsx Outdated Show resolved Hide resolved
Comment on lines 537 to 538
165 +
28 +
Copy link
Member

Choose a reason for hiding this comment

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

more magic numbers

Copy link
Member Author

Choose a reason for hiding this comment

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

Will take care of these in a follow-up PR

src/components/DappBrowser/BrowserTab.tsx Outdated Show resolved Hide resolved
src/components/DappBrowser/control-panel/ControlPanel.tsx Outdated Show resolved Hide resolved
src/components/DappBrowser/control-panel/ControlPanel.tsx Outdated Show resolved Hide resolved
src/components/DappBrowser/control-panel/ControlPanel.tsx Outdated Show resolved Hide resolved
brunobar79 and others added 2 commits April 27, 2024 13:23
* fix screenshot prunning

* remove logs

* Avoid subscribing to browserStore

---------

Co-authored-by: Christian Baroni <7061887+christianbaroni@users.noreply.github.com>
Copy link

socket-security bot commented Apr 30, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@candlefinance/faster-image@1.5.0 None 0 528 kB 6ary
npm/react-fast-memo@2.0.1 None 0 9.57 kB romgrk

🚮 Removed packages: npm/@candlefinance/faster-image@1.4.3

View full report↗︎

@christianbaroni christianbaroni marked this pull request as ready for review April 30, 2024 16:22
christianbaroni and others added 7 commits April 30, 2024 12:30
* network switch fixes

* Update src/components/DappBrowser/control-panel/ControlPanel.tsx

Co-authored-by: Christian Baroni <7061887+christianbaroni@users.noreply.github.com>

---------

Co-authored-by: Christian Baroni <7061887+christianbaroni@users.noreply.github.com>
Copy link
Member

@brunobar79 brunobar79 left a comment

Choose a reason for hiding this comment

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

🚀 🚀 🚀

@christianbaroni christianbaroni merged commit d4df288 into develop May 1, 2024
5 of 6 checks passed
@christianbaroni christianbaroni deleted the @christian/browser-new-architecture branch May 1, 2024 00:12
BrodyHughes added a commit that referenced this pull request May 3, 2024
…estro-test

* 'develop' of github.com:rainbow-me/rainbow:
  createRainbowStore (#5689)
  Browser: new architecture (#5671)
  handle injection natively (#5677)
  fix wallet balance in send flow (#5665)
  bump version to v1.9.23 (#5668)
  use current safari UA (#5670)
  Browser: browserStore (#5669)
BrodyHughes added a commit that referenced this pull request May 7, 2024
…eplink-add

* 'develop' of github.com:rainbow-me/rainbow: (22 commits)
  disable sentry (#5707)
  fix favorites list issues (#5659)
  fix: learn more webview clipped (#5687)
  add explorer_url and explorer_label to txn types and consume (#5690)
  Add names to wallet list info in backups v2 (#5692)
  feat: upgrading swap sdk to 0.19.0 (#5694)
  Add documentation (#5701)
  Fix supported chains bugs (#5697)
  useAnimatedTime, useAnimatedInterval, useAnimatedTimeout (#5699)
  hey (#5686)
  browser bugs (#5695)
  useSharedValueState (#5698)
  Connect Panel improvements (#5693)
  createRainbowStore (#5689)
  Browser: new architecture (#5671)
  handle injection natively (#5677)
  fix wallet balance in send flow (#5665)
  bump version to v1.9.23 (#5668)
  use current safari UA (#5670)
  Browser: browserStore (#5669)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants