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

Component lifecycle events #283

Closed
ryangjchandler opened this issue Mar 17, 2020 · 13 comments
Closed

Component lifecycle events #283

ryangjchandler opened this issue Mar 17, 2020 · 13 comments

Comments

@ryangjchandler
Copy link
Contributor

ryangjchandler commented Mar 17, 2020

Still working on a PR for component events and general events, but wanted to get some event ideas from everyone too.

Currently, I'm thinking about the following:

  • Before component data is mutated
  • After component data is mutated
  • Before component is refreshed
  • After component is refreshed
  • After Alpine has finished loading

The other question of course would be which element does the event get fired on. Logically, I think it should be the parent component element, but there might be an argument for firing the event at the document level too and then giving the user the parent component element so that they can do their own conditional logic.

Would be great to get some ideas from @SimoTod @HugoDF or @calebporzio :)

@ryangjchandler
Copy link
Contributor Author

We might want to add data specific events too. So when foo gets updated, an event gets fired.

@HugoDF
Copy link
Contributor

HugoDF commented Mar 17, 2020

With $watch and x-init do we still need the events to be emitted from Alpine core.

@ryangjchandler
Copy link
Contributor Author

With $watch and x-init do we still need the events to be emitted from Alpine core.

Can you do multiple '$watch's from the init directive? If you can, it would get quite messy I think. With the core events, you'd be able to register event listeners from outside of the component in your own JavaScript.

@ahmedkandel
Copy link
Contributor

ahmedkandel commented Mar 17, 2020

$watch callback is fired directly after the property mutations which means before the component update and component render/refresh. While $nextTick expression is executed after the component update but still before component render.

I know we can set a setTimeout in the callback to create a new task in JS stack after rendering, But I think having lifecycle events is a better choice.

e.g. if we want to scroll node.scrollTop = node.scrollHeight after component renders currently is not possible.

@SimoTod
Copy link
Collaborator

SimoTod commented Mar 17, 2020

I think event will be more expressive than watchers, i like that idea. Also, there is an annoying bug at the moment, Alpine just match the watcher by name so if you have the same name on different level, it just gets confused. Due to the why the membrane works, it doesn't seem easily fixable.

I would add events for transition stages as well, it's currently impossible to run something after a transition is completed, I think.

Separate topic, sorry. @ahmedkandel I think your PR fixes $nextTick as well. I believe the stack was emptied earlier because the debounce function wasn't working but now it should wait for the whole component to be updated (and rendered). I'll check after dinner.

@ryangjchandler
Copy link
Contributor Author

@SimoTod, do you have any more suggestions for events then? And any preference or suggestion on where the event should be fire, on the component or at a document level?

@SimoTod
Copy link
Collaborator

SimoTod commented Mar 17, 2020

I think component and then they will automatically bubble up to document and window but i haven't thought about that too much. I trust your decisions.

I think the events you mentioned are a solid base. There was an old ticket were we were talking about transition and we ended up saying that havin an event firing when a transition was completed would be a good but it was just an idea more than a real need. I'll send you the link if I can find it.

@ryangjchandler
Copy link
Contributor Author

I think component and then they will automatically bubble up to document and window but i haven't thought about that too much. I trust your decisions.

I think the events you mentioned are a solid base. There was an old ticket were we were talking about transition and we ended up saying that havin an event firing when a transition was completed would be a good but it was just an idea more than a real need. I'll send you the link if I can find it.

Perfect. I'll get cracking.

@SimoTod
Copy link
Collaborator

SimoTod commented Mar 17, 2020

#170 (comment) just a couple of messages in the middle of a wider issue but that one was the use case.

@ahmedkandel
Copy link
Contributor

Sorry, I am going off-topic but it is related.

@SimoTod

I think your PR fixes $nextTick as well. I believe the stack was emptied earlier because the debounce function wasn't working...

I thought the same thing but after checking I found another issue on $nextTick #287

@ryangjchandler the events feature will be 🔥

@ryangjchandler
Copy link
Contributor Author

100%. I'll hopefully have this PR ready and submitted by the end of the week since I'm working from home :)

@ryangjchandler
Copy link
Contributor Author

ryangjchandler commented Mar 17, 2020

Looking for some more input here guys, specifically on the naming conventions for the events. Similar to Livewire, I'm prefix all Alpine related events with alpine:, but how should the suffix be formatted.

For example, when a piece of data is updated, I've currently got alpine:data-updated as the event name. Any ideas or thoughts on this, as well as names for the other events that would make it clearer to users.

@SimoTod
Copy link
Collaborator

SimoTod commented Mar 17, 2020

As long as events are consistent and documented in the readme, I think they'll be fine.
If Caleb doesn't like those names when he reviews the PR, you will be able to change them quickly.

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

Successfully merging a pull request may close this issue.

4 participants