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

fix(AppRoot): Use memo for AppRootContext value #7667

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

andrey-medvedev-vk
Copy link
Contributor

@andrey-medvedev-vk andrey-medvedev-vk commented Sep 26, 2024

  • Release notes

Описание

Выяснилось, что если AppRoot по какой-то причине ререндерится, то это вызывает ререндер компонентов, основанных на Tappable/Clickable компонентах, даже если они обернуты в React.memo.
Виной тому пересоздающийся объект контекста.

Когда может ререндерится AppRoot?
Например, когда он обернут в компонент, который слушает изменения стора, бывает нужно для того, чтобы менять тему самого AppRoot. Тогда получается, что каждое обновление стора вызывает ререндер AppRoot и компонентов VKUI.

Пример с вспроизведением: https://7zmxhp.csb.app/

Нужно посмотреть что происходит в react profiler при каждом ререндере AppRoot.
До После
Screenshot 2024-09-26 at 16 17 16 Screenshot 2024-09-26 at 16 46 16
Код:
import React, { FunctionComponent, memo, useEffect, useState } from 'react'
import { createRoot } from 'react-dom/client'
import { AppRoot, Button } from '@vkontakte/vkui'

const rootContainer = document.getElementById('root')

if (rootContainer === null) {
  throw new Error('React #root element does not exist')
}

const root = createRoot(rootContainer, { identifierPrefix: 'vkm' })

root.render(<App />)

function App() {
  const [_, setState] = useState(0)

  useEffect(() => {
    setInterval(() => {
      setState((state) => ++state)
    }, 1000)
  }, [])

  return (
    <AppRoot>
      <MyButtons isVKUI />
      <MyButtons />
    </AppRoot>
  )
}

const MyButtons: FunctionComponent<{ isVKUI?: boolean }> = memo(
  function MyButtons({ isVKUI }) {
    return isVKUI ? (
      <div>
        <Button>VKUI button 1</Button>
        <br />
        <Button>VKUI button 2</Button>
        <br />
      </div>
    ) : (
      <div>
        <button>not VKUI button 1</button>
        <br />
        <button>not VKUI button 2</button>
        <br />
      </div>
    )
  },
)

Изменения

Используем React.useMemo для объекта, который мы передаем как значение контекста AppRootContext.

Release notes

Исправления

  • AppRoot: мемоизируем контекст AppRootContext, чтобы компоненты, зависящие от него, лишний раз не ререндерелись при ререндере AppRoot.

Otherwize even with memo any components based on Tappable/Clickable
will rerender on parent rerender.
@andrey-medvedev-vk andrey-medvedev-vk self-assigned this Sep 26, 2024
@andrey-medvedev-vk andrey-medvedev-vk added performance cmp:app-root patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча v6 Автоматизация: PR продублируется в ветку v6 labels Sep 26, 2024
@andrey-medvedev-vk andrey-medvedev-vk added this to the v6.8.0 milestone Sep 26, 2024
Copy link
Contributor

github-actions bot commented Sep 26, 2024

size-limit report 📦

Path Size
JS 385.35 KB (+0.01% 🔺)
JS (gzip) 116.62 KB (+0.02% 🔺)
JS (brotli) 95.79 KB (+0.01% 🔺)
JS import Div (tree shaking) 1.46 KB (0%)
CSS 331.15 KB (0%)
CSS (gzip) 41.51 KB (0%)
CSS (brotli) 32.9 KB (0%)

Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Copy link
Contributor

e2e tests

Playwright Report

Copy link
Contributor

github-actions bot commented Sep 26, 2024

👀 Docs deployed

Commit 06fdff3

Copy link

codecov bot commented Sep 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.16%. Comparing base (9ac63e2) to head (06fdff3).
Report is 12 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7667      +/-   ##
==========================================
+ Coverage   95.13%   95.16%   +0.03%     
==========================================
  Files         383      383              
  Lines       11347    11335      -12     
  Branches     3726     3714      -12     
==========================================
- Hits        10795    10787       -8     
+ Misses        552      548       -4     
Flag Coverage Δ
unittests 95.16% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andrey-medvedev-vk andrey-medvedev-vk marked this pull request as ready for review September 26, 2024 13:47
@andrey-medvedev-vk andrey-medvedev-vk requested a review from a team as a code owner September 26, 2024 13:47
Copy link
Contributor

@inomdzhon inomdzhon left a comment

Choose a reason for hiding this comment

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

👍

@andrey-medvedev-vk andrey-medvedev-vk merged commit 3ab56f7 into master Sep 27, 2024
53 checks passed
@andrey-medvedev-vk andrey-medvedev-vk deleted the mendrew/AppRoot/add-value-memoization branch September 27, 2024 07:46
vkcom-publisher pushed a commit that referenced this pull request Sep 27, 2024
Выяснилось, что если AppRoot по какой-то причине ререндерится, то это вызывает ререндер компонентов, основанных на Tappable/Clickable компонентах, даже если они обернуты в React.memo.
Виной тому пересоздающийся объект контекста.

Когда может ререндерится AppRoot?
Например, когда он обернут в компонент, который слушает изменения стора, бывает нужно для того, чтобы менять тему самого AppRoot. Тогда получается, что каждое обновление стора вызывает ререндер AppRoot и компонентов VKUI.
vkcom-publisher pushed a commit that referenced this pull request Sep 27, 2024
Выяснилось, что если AppRoot по какой-то причине ререндерится, то это вызывает ререндер компонентов, основанных на Tappable/Clickable компонентах, даже если они обернуты в React.memo.
Виной тому пересоздающийся объект контекста.

Когда может ререндерится AppRoot?
Например, когда он обернут в компонент, который слушает изменения стора, бывает нужно для того, чтобы менять тему самого AppRoot. Тогда получается, что каждое обновление стора вызывает ререндер AppRoot и компонентов VKUI.
@vkcom-publisher
Copy link
Contributor

v6.7.2 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmp:app-root patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча performance v6 Автоматизация: PR продублируется в ветку v6
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants