element-hq / element-hq/hydrogen-web

Side-effects should run after mount/`render` instead of immediately

Open
#1,069 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
714
Forks
131
PR merge metrics
No merged PRs in 30d

Description

Currently when you `t.mapSideEffect(...)`, it will run the side-effect immediately and whenever the binding value changes.

Current code:

https://github.com/vector-im/hydrogen-web/blob/915ab9c683897e3e2665ea68a55aa46a4e1536e4/src/platform/web/ui/general/TemplateView.ts#L374-L386

This leads to problems where if you want to manipulate a DOM element's focus or scroll (something you can't control with an attribute binding) or get element dimensions, you end up having to use hacks to make sure they happen after the component mounts. This should just be the sane-default.

Running the side-effects after "render"/"mount" is also inline with how React's `useEffect` works, see https://react.dev/learn/synchronizing-with-effects

It would also be nice to adapt the [clean-up function paradigm from `useEffect`](https://react.dev/learn/synchronizing-with-effects#step-3-add-cleanup-if-needed) as well so you can do easy clean-up when the component unmounts.

### Workaround

This works to get things running after the initial render but also makes all of your binding value changes run a frame later as well.

```js
t.mapSideEffect(vm => vm.shouldFocus, (shouldFocus, oldShouldFocus) => {
// assume this view will be mounted in the parent DOM straight away
requestAnimationFrame(() => {
if(shouldFocus) {
node.focus();
}
});
});
```

Here are example hacks straight from the Hydrogen codebase to get around this problem:

- [`src/platform/web/ui/session/room/LightboxView.js#L88-L90` -> `trapFocus(...)`](https://github.com/vector-im/hydrogen-web/blob/915ab9c683897e3e2665ea68a55aa46a4e1536e4/src/platform/web/ui/session/room/LightboxView.js#L88-L90)
- [`src/platform/web/ui/session/room/TimelineView.ts#L73-L77` -> `restoreScrollPosition()`](https://github.com/vector-im/hydrogen-web/blob/915ab9c683897e3e2665ea68a55aa46a4e1536e4/src/platform/web/ui/session/room/TimelineView.ts#L73-L77)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.