(preact-iso) - avoid calling onLoadEnd on every render #864
Conversation
🦋 Changeset detectedLatest commit: 7e28300 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
packages/preact-iso/router.js
Outdated
| @@ -176,7 +178,8 @@ export function Router(props) { | |||
|
|
|||
| // The route is loaded and rendered. | |||
| if (wasPush) scrollTo(0, 0); | |||
| if (props.onLoadEnd) props.onLoadEnd(url); | |||
| if (props.onLoadEnd && isLoading.current) props.onLoadEnd(url); | |||
| isLoading.current = false; | |||
| }); | |||
There was a problem hiding this comment.
The bigger issue is here as we're basically calling this layout-effect on ever single render rather than when it's needed, as this effect is inherently built on refs it's hard to give a reliable deps array, a reliable way would be to use a state for the loading times
There was a problem hiding this comment.
Hmm - that behavior is intentional though - the effect callback runs after every render to check whether that render suspended or not.
There was a problem hiding this comment.
Yes but suspending should be part of the observed state else you're hoping that the render cycle of your top-level router ties in with your suspended boundary, this implies that your useReducer and lazy.setState happen correctly
|
Size Change: +126 B (+2%) Total Size: 6.32 kB
ℹ️ View Unchanged
|
|
If we land this, I feel like we should also land a callback that fires after every successful route (sync or async), like |
Fixes #836
Ref solution: d6c87e3 (ties into the effect on every render)
State solution: 9603647 (avoids using effect on every render)