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(plasma-core, plasma-hope): Add promise resolve for forceUpdate method in Popup and Tooltip components #554

Merged
merged 1 commit into from
Jun 13, 2023

Conversation

neretin-trike
Copy link
Collaborator

@neretin-trike neretin-trike commented Jun 8, 2023

Исправлено поведение компонентов Tooltip и Popup, при использовании которых при изменении стейта возникала ошибка: Warning: flushSync was called from inside a lifecycle method. React cannot flush when React is already rendering. Consider moving this call to a scheduler task or micro task.

Это связано с тем, что в новой версии библиотеки popper метод forceUpdate содержит в себе flushSync и приводит к повторному рендеру компонента, который уже находится в процессе рендера.

🐤 Download canary assets:

color_sberHealth_ios-swift--canary.554.5210791882.swift
color_sberHealth_kotlin--canary.554.5210791882.kt
color_sberHealth_react-native--canary.554.5210791882.ts
color_sbermarket_business_ios-swift--canary.554.5210791882.swift
color_sbermarket_business_kotlin--canary.554.5210791882.kt
color_sbermarket_business_react-native--canary.554.5210791882.ts
color_sbermarket_ios-swift--canary.554.5210791882.swift
color_sbermarket_kotlin--canary.554.5210791882.kt
color_sbermarket_metro_ios-swift--canary.554.5210791882.swift
color_sbermarket_metro_kotlin--canary.554.5210791882.kt
color_sbermarket_metro_react-native--canary.554.5210791882.ts
color_sbermarket_react-native--canary.554.5210791882.ts
color_sbermarket_selgros_ios-swift--canary.554.5210791882.swift
color_sbermarket_selgros_kotlin--canary.554.5210791882.kt
color_sbermarket_selgros_react-native--canary.554.5210791882.ts
color_sbermarket_wlbusiness_ios-swift--canary.554.5210791882.swift
color_sbermarket_wlbusiness_kotlin--canary.554.5210791882.kt
color_sbermarket_wlbusiness_react-native--canary.554.5210791882.ts
color_sberonline_ios-swift--canary.554.5210791882.swift
color_sberonline_kotlin--canary.554.5210791882.kt
color_sberonline_react-native--canary.554.5210791882.ts
color_sberprime_ios-swift--canary.554.5210791882.swift
color_sberprime_kotlin--canary.554.5210791882.kt
color_sberprime_react-native--canary.554.5210791882.ts
shadow_sbermarket_react-native--canary.554.5210791882.ts
typo_mage_ios-swift--canary.554.5210791882.swift
typo_mage_kotlin--canary.554.5210791882.kt
typo_mage_react-native--canary.554.5210791882.ts
typo_plasma_ios-swift--canary.554.5210791882.swift
typo_plasma_kotlin--canary.554.5210791882.kt
typo_plasma_react-native--canary.554.5210791882.ts
typo_ruler_ios-swift--canary.554.5210791882.swift
typo_ruler_kotlin--canary.554.5210791882.kt
typo_ruler_react-native--canary.554.5210791882.ts
typo_sage_ios-swift--canary.554.5210791882.swift
typo_sage_kotlin--canary.554.5210791882.kt
typo_sage_react-native--canary.554.5210791882.ts
typo_sbermarket_ios-swift--canary.554.5210791882.swift
typo_sbermarket_kotlin--canary.554.5210791882.kt
typo_sbermarket_react-native--canary.554.5210791882.ts
typo_soulmate_ios-swift--canary.554.5210791882.swift
typo_soulmate_kotlin--canary.554.5210791882.kt
typo_soulmate_react-native--canary.554.5210791882.ts

📦 Published PR as canary version: Canary Versions

✨ Test out this PR locally via:

npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
# or 
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]

Version

Published prerelease version: @salutejs/[email protected]
@salutejs/[email protected]
@salutejs/[email protected]
@salutejs/[email protected]
@salutejs/[email protected]
@salutejs/[email protected]
@salutejs/[email protected]
@salutejs/[email protected]
@salutejs/[email protected]
@salutejs/[email protected]
@salutejs/[email protected]
@salutejs/[email protected]
@salutejs/[email protected]
@salutejs/[email protected]

Changelog

🚀 Enhancement

  • feat(plasma-tokens-native): Add generate complex gradient for iOS #548 ([email protected])
  • @salutejs/plasma-tokens-utils
    • feat(plasma-tokens-utils): Add support new format tokens value to generate ([email protected])

🐛 Bug Fix

  • ci: delete workflow for react 17 #544 (@Yakutoc)
  • ci: disable rebase for renovate #550 (@Yeti-or)
  • ci: add cache for perftool tasks #519 (@akhdrv)
  • @salutejs/plasma-core, @salutejs/plasma-hope
    • fix(plasma-core): Add promise resolve for forceUpdate method in Popup and Tooltip components #554 ([email protected])
  • @salutejs/plasma-tokens
    • docs: delete unnecessary mention in readme files #543 (@Yakutoc)

⚠️ Pushed to dev

  • chore: delete examples packages (@Yakutoc)
  • ci: delete unnecessary examples workflows (@Yakutoc)
  • ci: delete unnecessary unit test/lint workflow (@Yakutoc)
  • ci: extend unit test/lint workflow (@Yakutoc)

Authors: 4

@neretin-trike neretin-trike force-pushed the neretinaaa/fix-force-update-popper branch from 9e76cb4 to 809ce86 Compare June 8, 2023 11:44
@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-554/

@neretin-trike neretin-trike changed the title fix(plasma-core): Add promise resolve for forceUpdate method in Popup and Tooltip components fix(plasma-core, plasma-hope): Add promise resolve for forceUpdate method in Popup and Tooltip components Jun 8, 2023
@Salute-Eva
Copy link
Contributor

@Salute-Eva
Copy link
Contributor

Component performance testing

Result: 🟢 Success

Check out report in job artifacts!

* Данный хак, нужен для того, чтобы это поведение избежать и перенаправить
* вызов метода в очередь микрозадач.
*/
Promise.resolve().then(forceUpdate);
Copy link
Collaborator

Choose a reason for hiding this comment

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

[Уточнение]

@neretin-trike

forceUpdate появился в этой задаче - #461

Эти изменения регресс не принесли?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

не должны были. Этот метод всё-равно выполняется, но теперь в правильном порядке с точки зрения событийного цикла реакта

/*
* INFO: Метод forceUpdate содержит в себе flushSync и приводит
* к повторному рендеру компонента, который уже находится в процессе рендера.
* Данный хак, нужен для того, чтобы это поведение избежать и перенаправить
Copy link
Contributor

Choose a reason for hiding this comment

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

весело конечно

@neretin-trike neretin-trike added this pull request to the merge queue Jun 13, 2023
Merged via the queue into dev with commit 37f2d93 Jun 13, 2023
@neretin-trike neretin-trike deleted the neretinaaa/fix-force-update-popper branch June 13, 2023 09:12
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.

5 participants