(fix) O3-5555: Improve performance and readability of useLayoutType hook by AshThe25 · Pull Request #1702 · openmrs/openmrs-esm-core

AshThe25 · 2026-03-26T07:47:44Z

Requirements

This PR has a title that briefly describes the work done

Summary

This PR improves the performance of the useLayoutType hook by preventing unnecessary state updates during window resize events.

Changes

Added comparison before updating layout state to avoid redundant re-renders
Added debounce to resize handler to reduce excessive updates
Improved cleanup of event listeners

Screenshots

N/A

Related Issue

https://issues.openmrs.org/browse/O3-5555

Note

This is a small improvement to prevent silent null returns and improve API safety. No existing JIRA issue was found for this case.

Other

N/A

changeset-bot · 2026-03-26T07:47:50Z

⚠️ No Changeset found

Latest commit: e4c0cff

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

AshThe25 · 2026-03-26T07:49:49Z

In the course of doing this, I also realized that the resize handler may cause excessive updates even when the layout hasn’t changed.

This ensures that the states are updated only when there is an actual layout change, thus reducing the number of re-renders.

Also, the inclusion of the debounce prevents excessive updates during rapid resizes, thus improving performance.

Nandalily · 2026-03-29T03:13:46Z

This is a nice improvement @AshThe25, reducing unnecessary re-renders and de-bouncing resize events should significantly improve performance I think it might also be helpful to add a test to verify layout state updates, to detect only when the layout actually changes and not on every resize event

AshThe25 · 2026-03-29T04:14:34Z

This is a nice improvement @AshThe25, reducing unnecessary re-renders and de-bouncing resize events should significantly improve performance I think it might also be helpful to add a test to verify layout state updates, to detect only when the layout actually changes and not on every resize event

Hey thanks a lot this actually means a lot for lets connect sometime on linkdin new here and if made any mistake do lemme know will surely work on it and thanks for that feedback really means a lot

Ratangulati · 2026-03-31T16:41:49Z

Hey @AshThe25, just a heads up, PR #1707 by @dulajweligaththa08 addresses the same useLayoutType hook with very similar changes, including the debounce and redundant state update prevention. It would be worth coordinating with that PR to avoid duplicated effort or a merge conflict.

ibacher

Thanks for this submission. In the absence of data supporting that this fixes a genuine problem (e.g., a test or something), this seems a little excessive. I also think the analysis here is misguided in that the attempted de-duplication really isn't effective. Please try to produce a test case that will show the value of this addition and remove the redundant redundancy check.

ibacher · 2026-04-01T14:03:48Z

+      setType((prev) => {
+        if (prev === newLayout) {
+          return prev;
+        }
+        return newLayout;
+      });


While I get the idea of trying to deduplicate things, it's important to track what this value is. Fundamentally, it's a string and since 'large-desktop' === 'large-desktop', this kind of guard is purely redundant. Rather than improving things, this makes the code more complicated for no actual gain.

ibacher · 2026-04-01T14:04:18Z

+        clearTimeout(resizeTimeout.current);
+      }
+
+      resizeTimeout.current = setTimeout(updateLayout, 100);


AshThe25 · 2026-04-19T17:24:05Z

Hello @ibacher @Nandalily! I’m sorry for not getting back sooner; had an accident recently and had to take some time off. But I’m back now!

You guys are completely correct about everything. I made some changes in the code in the most recent commit:

Got rid of the prev === newLayout check. I didn’t need it because of React’s useState; when you use primitives for state, it doesn’t update anything when the value hasn’t changed. Got rid.
Justified the debounce delay. Extracted it into a const RESIZE_DEBOUNCE_MS = 150 and added the justification why it should be 150 milliseconds: the browser triggers the resize event up to ~60 times per second, which means without debouncing, getLayout() will trigger on each call. So it would check the DOM’s classList every second. 150ms will skip all the intermediate breakpoints and won’t make a difference to the user.
Added some tests for the timer behavior with vi.useFakeTimers(), including:
Check that layout doesn’t change before debounce settles.
Debounce works — fast resizing collapses to the last breakpoint.
If you think 150ms isn’t a good number, just let me know.

AshThe25 · 2026-04-19T19:33:19Z

@ibacher, I wanted to confirm that the last commit resolved your concerns.
Eliminated the unnecessary comparison of states, extracted the debounce
interval to a constant, explained why I chose 150ms in the comments,
and included tests using vi.useFakeTimers(). Let me know if there’s
anything else you need changed.

AshThe25 · 2026-04-20T18:19:32Z

Hi @ibacher, addressed your feedback:

Removed the prev === newLayout comparison entirely — you're right that React's useState already skips re-renders when the primitive value hasn't changed
Extracted the debounce interval to RESIZE_DEBOUNCE_MS = 150
Added tests using vi.useFakeTimers() that demonstrate: (1) layout does not update until debounce settles, (2) rapid resize events collapse to a single update, (3) pending timeout is cancelled on unmount
Restored the /** @module @category UI */ JSDoc that was accidentally dropped

All 13 tests in useLayoutType.test.tsx pass.

AshThe25 · 2026-04-20T21:58:04Z

@ibacher all your feedback from the Apr 1 review has been addressed in the latest commit:

Removed the prev === newLayout state comparison (React's useState already skips re-renders for unchanged primitives)
Extracted debounce interval to RESIZE_DEBOUNCE_MS = 150 constant with a comment explaining the rationale
Added 13 tests with vi.useFakeTimers() covering: debounce delay, burst collapsing, cleanup on unmount

SonarQube Quality Gate passed (0 new issues). Could you take another look when you get a chance? Thanks!

@module

…undant updates Browser fires resize up to ~60 times/s during a drag. Without debouncing, getLayout() re-reads the DOM on every event. Debouncing at 150ms collapses rapid bursts to a single read after the resize settles. - Add RESIZE_DEBOUNCE_MS = 150 constant - Debounce the resize handler using useRef to hold the timeout - Cancel pending timeout on unmount to prevent stale state updates - Restore accidentally removed /** @module @category UI */ JSDoc - Add tests with vi.useFakeTimers() covering debounce delay, burst collapsing, and cleanup on unmount

sonarqubecloud · 2026-04-20T22:58:36Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

AshThe25 · 2026-04-20T22:58:46Z

@ibacher added a comment above RESIZE_DEBOUNCE_MS explaining why 150 ms — browsers fire resize at up to ~60 fps, so without debouncing getLayout() hits the DOM every ~16 ms during a drag. 150 ms skips transient breakpoints while still feeling instant to users. SonarCloud green.

AshThe25 changed the title ~~refactor(useLayoutType): prevent unnecessary re-renders and debounce …~~ refactor(esm-react-utils): improve useLayoutType performance and readability (O3-5555) Mar 26, 2026

AshThe25 changed the title ~~refactor(esm-react-utils): improve useLayoutType performance and readability (O3-5555)~~ O3-5555: Improve performance and readability of useLayoutType hook Mar 26, 2026

AshThe25 changed the title ~~O3-5555: Improve performance and readability of useLayoutType hook~~ (refactor) O3-5555: Improve performance and readability of useLayoutType hook Mar 26, 2026

AshThe25 changed the title ~~(refactor) O3-5555: Improve performance and readability of useLayoutType hook~~ (fix) O3-5555: Improve performance and readability of useLayoutType hook Mar 26, 2026

ibacher mentioned this pull request Apr 1, 2026

improve performance and readability of useLayoutType hook #1707

Closed

ibacher requested changes Apr 1, 2026

View reviewed changes

AshThe25 requested a review from ibacher April 19, 2026 17:24

AshThe25 force-pushed the fix/useLayoutType-performance branch 2 times, most recently from 733e08b to 0427121 Compare April 20, 2026 18:19

AshThe25 force-pushed the fix/useLayoutType-performance branch from 0427121 to e4c0cff Compare April 20, 2026 22:57

Conversation

AshThe25 commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Requirements

Summary

Screenshots

Related Issue

Note

Other

Uh oh!

changeset-bot bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

AshThe25 commented Mar 26, 2026

Uh oh!

Nandalily commented Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AshThe25 commented Mar 29, 2026

Uh oh!

Ratangulati commented Mar 31, 2026

Uh oh!

ibacher left a comment

Choose a reason for hiding this comment

Uh oh!

ibacher Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

ibacher Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

AshThe25 commented Apr 19, 2026

Uh oh!

AshThe25 commented Apr 19, 2026

Uh oh!

AshThe25 commented Apr 20, 2026

Uh oh!

AshThe25 commented Apr 20, 2026

Uh oh!

sonarqubecloud bot commented Apr 20, 2026

Quality Gate passed

Uh oh!

AshThe25 commented Apr 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AshThe25 commented Mar 26, 2026 •

edited

Loading

changeset-bot bot commented Mar 26, 2026 •

edited

Loading

Nandalily commented Mar 29, 2026 •

edited

Loading