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

RFC: onMount* callbacks should fire even if the element is already mounted #149

Open
raquo opened this issue Jan 15, 2024 · 0 comments
Open

Comments

@raquo
Copy link
Owner

raquo commented Jan 15, 2024

Problem

Callbacks like onMountCallback, onMountInsert, onMountBind all fire when the element is mounted, quite literally: If you already have an element, and it's already mounted, and you say el.amend(onMountCallback(callback)), callback won't actually be run, because you missed the mounting.

At first glance, this might make sense given the naming, but I don't know if that's actually ergonomic.

Typically we add onMount* modifiers right when constructing the element – and at this point we know that the element is not yet mounted, so we know that the mount callbacks will run, and the question is moot.

But, even in those cases, we might be calling into our own helper methods that return modifiers that use onMountInsert internally – and so those helpers / modifiers have an hidden precondition in them, that they only work as expected on elements that haven't been mounted yet.

Currently, if you want onMount* logic to run even if the logic is added when the component is already mounted, you don't really have a good alternative. You can do a simple callback with el.amend(EventStream.unit --> doSomething) but you don't get an Owner, and you don't get bracketing mount-unmount logic, the two main reasons to use onMount* methods in the first place.

Solution

I think we should make onMount* methods fire their onMount callback even when they're being added to a component that is already mounted.

This should be the default behaviour, and if you actually want the previous behaviour, you would specify an ignoreAlreadyMounted = true as the last argument.

(This isn't for v17, but for later.)

Request for Comments

I only ever seem to run into cases when the current behavior is undesirable.

Do you know of any use cases that actually benefit from the current behavior?

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

No branches or pull requests

1 participant