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: Maximum update depth exceeded error #1561

Closed
wants to merge 26 commits into from

Conversation

Dzuming
Copy link
Contributor

@Dzuming Dzuming commented Sep 13, 2024

  • The problem is, that sometimes (for example during resize as in the video below) in this place
 useLayoutEffect(() => {
     if (contentAvailable && position && !(maximized || wasMaximized)) {
       const newValue = calcEdgePosition(viewport);
       setPosition((current) => (isEqual(newValue, current) ? current : newValue));
     }
   }, [contentAvailable, wasMaximized, calcEdgePosition, maximized, position, viewport]);

setPosition falls into an infinite loop, i suspect that it can be a race condition, and since we have a dependencies position, the values differ

  • the issue was reported by a few people in different non-deterministic contexts, however here are the easiest steps to reproduce it

Before:

Screen.Recording.2024-09-13.at.13.05.27.mov

After:

Screen.Recording.2024-09-13.at.13.06.29.mov

Copy link
Contributor

created: #1562

Copy link

codecov bot commented Sep 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.57%. Comparing base (1d4b2e1) to head (76a3ec6).
Report is 38 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1561      +/-   ##
==========================================
+ Coverage   84.51%   84.57%   +0.06%     
==========================================
  Files          58       58              
  Lines         846      830      -16     
  Branches      168      165       -3     
==========================================
- Hits          715      702      -13     
+ Misses        127      124       -3     
  Partials        4        4              

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

@Dzuming Dzuming requested review from JulianWielga and removed request for JulianWielga September 13, 2024 12:10
Base automatically changed from dev to master September 16, 2024 09:36
@JulianWielga JulianWielga changed the base branch from master to dev September 16, 2024 09:45
Base automatically changed from dev to master September 16, 2024 10:47
@JulianWielga JulianWielga changed the base branch from master to dev September 16, 2024 11:18
Dzuming and others added 10 commits November 22, 2024 13:11
…-exceeded-error' into NU-1807-fix-Maximum-update-depth-exceeded-error
# [1.9.0](v1.8.0...v1.9.0) (2024-09-16)

### Bug Fixes

* Add nodeRef and fix Warning: findDOMNode is deprecated and will be removed for react-transition-group ([6683e24](6683e24))
* clicking outside when modal mode is disabled ([7ed67fb](7ed67fb))
* position of a window after full-screen zoom via title double click and zoom out via header icon ([1d4b2e1](1d4b2e1))
* position when a window is full height ([b822c7b](b822c7b))
* safari pinch-to-zoom crash ([1ebc843](1ebc843))
* support keyboard in header buttons ([66755de](66755de))
* type definitions ([e30bbc9](e30bbc9))
* window lazy loading animation ([5a14fcf](5a14fcf))
* window move issue ([2f3bde5](2f3bde5))

### Features

* moved window size props to layoutData ([d2e94c7](d2e94c7))
* react 18 support ([d4ad55d](d4ad55d))
* set initial position with layoutData ([c59adf7](c59adf7))
semantic-release-bot and others added 5 commits November 27, 2024 14:24
## [1.9.1-beta.1](v1.9.0...v1.9.1-beta.1) (2024-11-25)

### Bug Fixes

* issue with manually change window size ([9af367b](9af367b))
## [1.9.1-beta.2](v1.9.1-beta.1...v1.9.1-beta.2) (2024-11-27)

### Bug Fixes

* revert change animation library ([13736c3](13736c3))
…-exceeded-error' into NU-1807-fix-Maximum-update-depth-exceeded-error

# Conflicts:
#	package-lock.json
#	package.json
@Dzuming Dzuming closed this Nov 27, 2024
@Dzuming Dzuming deleted the NU-1807-fix-Maximum-update-depth-exceeded-error branch November 27, 2024 13:32
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.

4 participants