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

Let developers associate lifecycle events #764

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

domchristie
Copy link
Contributor

@domchristie domchristie commented Oct 13, 2022

Currently there is no way to know which events are tied to a given user action. For example, it's not possible to associate a turbo:click with a turbo:visit. One might link a turbo:click with the next turbo:visit, but this isn't accurate because a navigation can be ended by cancelling turbo:click or turbo:before-visit (as mentioned by @pfeiffer in #99 (comment)).

With a lifecycle ID present in each event, it'll be possible to reference the initiator element, providing a way for developers to customise visit behaviour based on HTML attributes. For example:

let initiator = []
addEventListener('turbo:click', ({ detail }) =>
  initiator = [detail.lifecycleIdentifier, detail.originalEvent.target]
)
addEventListener('turbo:visit', ({ detail }) => {
  const [id, element] = initiator
  if (detail.lifecycleIdentifier === id && element.dataset.animate) {
    // do something
  }
}) 

It could also be used to determine redirect visits, as turbo:visit is dispatched twice with the same lifecycle ID:

let lifecycleIdentifier
addEventListener('turbo:visit', ({ detail }) =>
  const isRedirect = detail.lifecycleIdentifier === lifecycleIdentifier
  lifecycleIdentifier = detail.lifecycleIdentifier
)

This is a lower-level alternative to #750, and whilst I still think that the type annotations in that PR are worthwhile, this might provide some useful information for other cases without having to manipulate VisitOptions too much.

If we think this approach is worth pursuing, I can finish it up by adding identifiers to the remaining events.


Todo:

  • Other events
  • Fix back/forward having same ID

@seanpdoyle
Copy link
Contributor

seanpdoyle commented Oct 13, 2022

Thank you for exploring this option @domchristie!

While I appreciate the simplicity of passing around a uuid string, I wonder how likely it is that applications that adopt this pattern are going to want more and more details about the details from events that precede any given action.

The hypothetical you shared is related to the lifecycle of a Visit instance. While I'd really prefer to not expose internal objects within CustomEvent.detail, I wonder if there is a middle ground between sharing the Visit instance across related events and sharing an abstract uuid string.

Would it be worth pushing each Event instance onto an Array? Capturing an object that adheres to a Delegate interface (maybe like VisitDelegate) during a turbo:click, then forwarding that along? Something else entirely?

@domchristie
Copy link
Contributor Author

Thanks for reviewing @seanpdoyle.

Yes, the idea for a just a UUID string stems from the issue of passing around complex objects that don't conform to the structured clone algorithm (discovered by @kevinmcconnell in #720), which caused an error when communicating with the iOS adapter.

Related: when augmenting the code, it's really inviting to add arbitrary properties to VisitOptions. Many of these make sense in terms of modelling (e.g. Visit#initiator:Element feels natural), and can be really handy because VisitOptions are passed around everywhere prior to a Visit, however, this is risky because of the above.

The other consideration is that there currently isn't a model that encompasses the pre-visit events like turbo:click. Perhaps this is a Lifecycle or Navigation?

@domchristie
Copy link
Contributor Author

Would it be worth pushing each Event instance onto an Array? Capturing an object that adheres to a Delegate interface (maybe like VisitDelegate) during a turbo:click, then forwarding that along? Something else entirely?

@seanpdoyle just to clarify, are you suggesting that this array of events (or alternative) be available in the Event object, or referencable via some other API, e.g Turbo.currentLifecycle.events?

@domchristie
Copy link
Contributor Author

domchristie commented Jul 12, 2023

I still feel this approach might be worth pursuing. If events share a UUID, then at least developers can create their own method of tracking the lifecycle.

It could really help with the issue described here: #935 (comment)

For example in turbo:before-render, apps could look up the click or submit-start event by UUID, and customise view transition behaviour based on the original event.target

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

Successfully merging this pull request may close these issues.

None yet

2 participants