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(cacheable-section): stable references to avoid loops [LIBS-642] #1385

Merged

Conversation

KaiVandivier
Copy link
Contributor

@KaiVandivier KaiVandivier commented Jul 18, 2024

Implements LIBS-642

These hooks were implemented unoptimally, admittedly, and functions were getting recreated on each render. This caused issues if they were used as dependencies for useEffect hooks, like an infinite loop demoed below

I've updated the hooks to use best practices with useCallback and useMemo to make stable references and minimize unnecessary rerenders, and I added some automated tests to help verify the stability -- the tests look simplistic, but they were failing before the changes, and actually helped track down which hooks were still unstable in a TDD way

I also tested manually with the PWA example app in the App Platform, with some code that looks like an implementation in analytics plugins:

useEffect(() => {
    if (shouldRecord) {
        startRecording()
    }
}, [shouldRecord, startRecording])

Demo of loop:

recording-loop-before.mov

After changes:

recording-loop-after.mov

@KaiVandivier KaiVandivier requested review from HendrikThePendric and a team July 18, 2024 21:40
@KaiVandivier KaiVandivier enabled auto-merge (squash) July 26, 2024 16:12
@KaiVandivier KaiVandivier merged commit e3a5fbf into master Jul 26, 2024
14 checks passed
@KaiVandivier KaiVandivier deleted the libs-642-stable-references-for-cacheable-sections branch July 26, 2024 16:22
dhis2-bot added a commit that referenced this pull request Jul 26, 2024
## [3.10.6](v3.10.5...v3.10.6) (2024-07-26)

### Bug Fixes

* **cacheable-section:** stable references to avoid loops [LIBS-642] ([#1385](#1385)) ([e3a5fbf](e3a5fbf))
@dhis2-bot
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants