Provide helper for keeping state while a component is attached

[Legioth]

It's a relatively common pattern to initialize some state when a component is attached and then do the reverse when the component is detached. One typical example of this is to register the UI for background updates to be pushed out. Another example is with using executeJs to initialize the client-side of a component but then wanting to cancel the registration on detach to avoid double init in case the component is attached and detached again during the same round trip.

The most straightforward way of implementing this is by overriding onAttach and onDetach.

public class MyComponent extends Div {
  private Registration subscribeRegistration;

  @Override
  protected void onAttach(AttachEvent attachEvent) {
    subscribeRegistration = registerForPush(this, attachEvent.getUI());
    super.onAttach(attachEvent);
  }

  @Override
  protected void onDetach(DetachEvent detachEvent) {
    super.onDetach(detachEvent);
    subscribeRegistration.remove();
    subscribeRegistration = null;
  }
}

There's a whole bunch of boilerplate code associated with this technique, and the logic does also suffer from lack of locality since the concerns are spread out over two separate instance methods and one instance field.

Another alternative is to use addAttachListener and addDetachListener to keep all logic in one method. Furthermore, unregisterListener can be used to create a one-time detach listener which in turn allows keeping the state local instead of relying on an instance field.

public class MyComponent extends Div {
  public MyComponent() {
    addAttachListener(attachEvent -> {
      Registration subscribeRegistration = registerForPush(this, attachEvent.getUI());

      addDetachListener(detachEvent -> {
        detachEvent.unregisterListener();
        subscribeRegistration.remove();
      });
    });
  }
}

There's still some boilerplate in this pattern, and it's also a pattern that isn't immediately obvious.

One additional challenge is in case the logic is outside the component itself. In this case, the same technique with addAttachListener can be used, but there's also the need for additional boilerplate to run the attach logic immediately in case the target component is already attached.

As an alternative, there could be a helper method either as an instance method in Component or as a static helper in ComponentUtil that takes an attach handler that is supposed to return a matching detach handler. The attach handler immediately receives the UI instance rather than an event object that needs unpacking. The helper would also take care of running the attach handler immediately in case the method is currently attached and it would finally also take care of running the detach handler if one is present in case the registration of the attach handler itself is removed.

The method signature could thus look like this if implemented as an instance method:

public Registration whileAttached(SerializableFunction<UI, Registration> attachHandler);

Usage would be like this in the happy case when the internal logic is also based on Registration.

public class MyComponent extends Div {
  public MyComponent() {
    whileAttached(ui -> registerForPush(this, ui));
  }
}

In case some other way of unregistering is needed, then it might be necessary to create an inline Registration to return.

public class MyComponent extends Div {
  public MyComponent() {
    whileAttached(ui -> {
      SomeType someInstance = registerSomething(this, ui);
      return () -> unregisterSomething(someInstance); 
    });
  }
}

In other cases, it might be enough to convert some other no-args callback into a Registration as a method reference by adding e.g. ::close to the end, e.g. return someClosableThing::close.

[knoobie]

I like the idea, especially combined with use cases like e.g. https://cookbook.vaadin.com/ui-eventbus - I'm wondering about the naming of whileAttached.. this whole concept sounds like a "lifecircle".. could this naming be used in this regard? Or is this name too broad?

[Legioth]

I'm also not sure about the naming. I'm treating whileAttached more as a discussion starter and less as a definite proposal.

I'm not sure about "lifecircle" either - it seems like a quite accurate description but it's on the other hand quite fluffy and seems to have some controversial associations outside the tech domain.

[pleku]

I'm not too fond about the naming whileAttached either as even if it is semantically correct, I would not have not search for it since I have not seen it elsewhere. Maybe something that would be closer to the existing addAttachListener could work.

@Tag("my-web-component")
public class MyWebcomponent extends Component {
  public MyComponent() {
    addUiAttachCallback(ui -> initializeClientSide());
  }
}

I would like it to be somehow explicit that the code is related to being attached to a new UI instance, so it could be used to fix all the issues related to @PreserveOnRefresh e.g. when the component instance stays the same but the initialization needs to happen again. But to follow the existing practice, it kind of should be an method you can override instead of a callback you add, but then again, probably the onAttach+onDetach methods suffer a bit from not being so easy to discover, as I see people use the listeners even inside the components (not intended this way). Or do everything inside the constructor.

Also, I don't like the way the detach callback is returned. Instead I would just add another API for it, or instead chain it explicitly like

public MessageTray extends Div {
    public MessageTray() {
        addUiAttachCallback(ui -> doStuff(ui, this))
                .addUiDetachCallback(ui -> stopDoingStuff(ui, this)); // or withUiDetachCallback
    }
}

but that is just personal preference, to me the API where you have to return something is not as it hides what the code is meant for. But the chaining is not really that much different from just explicitly calling Component::addUiDetachCallback. If the method name seems too long, it could maybe be onUiAttach(ui -> doStuff(ui, this)); but then it is again slightly different naming convention to what we have for similar things.

One big benefit to having another methods to override when used inside the component is the discoverability - I always see onAttach and onDetach for the specific logic related to attaching/detaching the component.

[Legioth]

chaining is not really that much different from just explicitly calling Component::addUiDetachCallback.

There's a massive difference: state. If the thing you're initializing returns a registration object or if you pass in a listener instance that you also need to use for removing, then you have to do explicit bookkeeping (i.e. an instance field) to make that same value available also in the detach handler. If just want to listen to an attach event and optionally also a separate detach event, then you can just as well use current APIs. The primary reason for the suggested API is that it lets you directly carry over state between a specific attach event and the corresponding detach event.

One big benefit to having another methods to override when used inside the component is the discoverability

The drawback is that overridden methods are only available inside the component. There are also some cases when you want to hook on to the life cycle of another component (e.g. much of CollaborationEngine). If we offer a public method, then it's available for all use cases. If we only offer a protected method to override, then we are limiting the benefits to only a subset of the cases. There might be a case for offering both, but I'm not sure that would be worthwhile for a helper like this.

I see people use the listeners even inside the components (not intended this way).

I use it because it's less code to type for effectively the same end result (the difference being slightly more allocated memory from the callback instance and the entry in a listener list). The listener API can often be a semantically descriptive oneliner, whereas the overridden method carries multiple lines of pure boilerplate and whitespace. I'm not sure if I would even include onAttach if I would design Flow today - I see it mostly as a relic from old versions of Vaadin that didn't have addAttachListener.

[Legioth]

An alternative way of reaching the same benefits would be to reuse the existing entry points (onAttach and addAttachListener), but supplement them to also support this particular use case.

Carrying over state from the attach event could be streamlined if AttachEvent would have a method for registering a corresponding one-time detach listener. The detach callback could be in the form of a Registration since the detach event isn't necessary when the same info is already in the attach event which remains in the closure.

Firing an initial event if the component is already attached would only be necessary for addAttachListener but not for onAttach. The most smooth way might be to return a custom Registration subtype that would allow chaining in the extra instruction, e.g. addAttachListener(listener).fireNowIfAttached(), but this would break binary compatibility. A safer alternative would be to introduce an overload that takes an enum signal that it should be fired immediately. A similar "eager" listener mode would also be useful with e.g. value change listeners.

Taken together, my initial example could then be expressed in this way:

someComponent.addAttachListener(event -> {
    SomeType someInstance = registerSomething(someComponent, ui);
    event.onNextDetach(() -> unregisterSomething(someInstance));
}, EventListenerMode.EAGER);

A case that uses a registration could be written as a oneliner, but it would be quite confusing since the attach action would be "inside" onNextAttach which might give the impression that it happens only when detaching:

addAttachListener(event -> event.onNextDetach(registerForPush(this, event.getUI()));

[Legioth]

Yet another commonly needed feature is related to cleanup. Adding a regular attach listener returns a registration that can be used to remove the attach listener. In this particular case where state needed for cleanup is captured inside the listener closure, it would also be necessary trigger the cleanup action on registration removal in case the component is currently attached.

This is how that aspect would work with the originally suggested API:

Registration registration = component.whileAttached(ui -> {
  System.out.println("Registering");
  return () -> System.out.println("Unregistering");
});

attachedComponent.add(component); // Prints "Registering"
registration.remove(); // Prints "Unregistering"
attachedComponent.remove(component); // Doesn't print anything

This unregistration behaviour wouldn't be natural for the alternative API suggestion based on event.onNextDetach since there's no logical connection between the attach listener and some action performed through an event passed to it.

This way of looking at the registration does on the other hand lead towards another potential API design direction by seeing this functionality as a Registration subclass rather than as a component feature:

Registration registration = new WhileAttachedRegistration(component, ui -> {
  System.out.println("Registering");
  return () -> System.out.println("Unregistering");
});

This approach sacrifices some discoverability but does instead remove the need of polluting the sacred Component namespace.

[Legioth]

I spotted yet another typical example where this would be useful: https://cookbook.vaadin.com/ui-eventbus

[Legioth]

Yet another use case: https://gist.github.com/OlliTietavainenVaadin/dbbec3ad7629b9292030b93d24d2dd33#file-helloworldview-java-L37-L49

[knoobie]

Isn't that over-engineered with the Dialog::setCloseOnEsc(true); API? ;)

[Legioth]

Workaround for the issue that pending changes in the TextArea won't be sent to the server with setCloseOnEsc(true).