Disconnected handler for directives

[simonbuchan]

I've been playing around with combining lit-html and observables, and it feels really nice!
The only issue I've run into is that, since observables are a push-based API, they must be explicitly unsubscribed when the source outlives the destination to avoid leaking events, memory etc....

The problem

Lets say I'm using a very simple directive:

const observeReplace = value$ => directive(part => {
  value$.subscribe(value => part.setValue(value));
});

And rendering it as such:

const second$ = Rx.Observable.interval(1000).do(console.log);

render(html`Seconds running: ${observeReplace(second$)}`, document.body);

Everything is fine. However if you later replace the template:

setTimeout(() => {
  render(html`Should be stopped.`, document.body);
}, 5000);

The interval subscription is still running, as the .do(console.log) shows, and lit-html errors:

Subscriber.js:214 Uncaught TypeError: Cannot read property 'nodeType' of null
    at NodePart._setText (lit-html.js:483)
    at NodePart.setValue (lit-html.js:449)
    at SafeSubscriber.value$.subscribe.value [as _next] (index.js:13)
    at SafeSubscriber.next (Subscriber.js:173)
    at Subscriber._next (Subscriber.js:118)
    at Subscriber.next (Subscriber.js:82)
    at TapSubscriber._next (tap.js:98)
    at TapSubscriber.next (Subscriber.js:82)
    at AsyncAction.dispatch [as work] (interval.js:56)
    at AsyncAction._execute (AsyncAction.js:105)

To fix this, a directive needs to provide a way to know when new values should stop being pushed to the part. So far the only way I know of to do this with the current api is to somehow use MutationObserver to detect part.instance.template.element being removed, but that requires that you observe childList mutations on the parent, which isn't present when the directive callback runs.

Possible solutions

The ideal for me would be to simply accept observables 😉 (which are basically equivalent to directives) but a possibly smaller change would be to just steal a trick from observable subscriber functions and have directive callbacks optionally return a cleanup function:

const observeReplace = value$ => directive(part => {
  const sub = value$.subscribe(value => part.setValue(value));
  return () => sub.unsubscribe();
});

// not observable specific:
const now = directive(part => {
  const id = setInterval(() => part.setValue(new Date().toString()), 1000);
  return () => clearInterval(id);
});

[justinfagnani]

Thanks for the thoughtful issue @simonbuchan!

There's a few interesting things to unpack here.

First, parts should possibly be resilient to having setValue() called when detached. I need to think some more about whether that should be a program error or not. Other opinions would help here.

Second, I believe we have a similar problem with async iterable support. We should add tests and confirm. We have code to stop listening to new values if the iterable it no longer the current iterable for a Part, but not if the Part or directive is simply removed: https://github.com/Polymer/lit-html/blob/master/src/lib/async-replace.ts#L63

Third, I hope that we can have just one streaming type to support. Do you know why converting the observable to an async iterable wouldn't work? We also have Streams showing up soon, and I don't want to triple-up code, especially when Observable aren't a platform type.

Fourth, while it'd be expensive and cumbersome to use MutationObservers to detect Part disconnection, we can use the Part system itself, assuming all structural mutations are done via a template. Directives are values, and we can do some action, like call a callback, when the directive it no longer the current value of a part. Parsed Parts themselves are never removed from a TemplateInstance, the whole TemplateInstance has to be removed, but we can know this most of the time and do some action there. Dynamic Parts, like for iterables, are explicitly removed too.

[simonbuchan]

It's safer to wrap an async iterator in an observable than vice versa, due to back-pressure: an observable could dispatch a million items synchronously, and the iterator wrapper would have to buffer them all, but in this use case that's not a huge deal.

I'm not for sure certain, but I think Streams are a superset of both, so that might work out.

In any case this issue is more about the fact that it's impossible for a late setValue() to be a program error right now, because it doesn't have any way to know that it should stop.

Mutation observers was just pointing out this is very hard to workaround right now.

I've actually been playing around with a cut down version of this to see how hard it is, and it's a bit of a mess. I think I nearly have a good long term design, (eg also handling weird combinations like iterables of observables in attributes) but it's basically a rewrite of the part system, so there's probably a better low hanging fruit.

[simonbuchan]

Also, when wrapping an observable in on async iterable, you must close the iterable with return so the subscription can be closed, so you have the same issue anyway.

[simonbuchan]

If you're interested, here's the fork I was talking about: https://github.com/simonbuchan/retemplate

I don't actually expect you to use anything from it: it's actually a pretty aggressive refactor and stripping down of just lit-html.ts - it's my habit for code I don't understand yet - sorry? I've probably broken lots of things, but in particular attribute parts and array/enumerables of promises.

The relevant bits for this issue though are:

• updateSlot() (slot=part - for now. I'm trying to figure out the multi part attribute case), which passes the value to
• getValueResolver(), which wraps it into an observable-ish of arrays of resolved values (empty, string, node or template).
• then uses the generic interface to handle updating the target with the new resolved values, including unsubscribing children.

There is example usage showing it automatically subscribing and unsubscribing to rxjs observables in https://github.com/simonbuchan/retemplate/blob/472022bc6fb9cd5f9517102b9395f726c9b63f27/examples/hello-world/src/index.ts

Regarding this issue, finding a way to do the moral equivalent of getValueResolver() in the current code is pretty easy - just have directive() optionally allow it's callback to return a cleanup function that gets called on unmount, as above, the more interesting problem is the equivalent of updateSlot, which is close to *Part.setValue() but has to know the difference between template re-rendering (which has to cleanup the previous template part value and stash the new cleanup function) and calls from the directive() which must not cleanup the current part value (or you will only get the first value!), but must still clean up removed template instances.

[justinfagnani]

I've started working on this. It won't make 1.0, but probably will 1.1.

Right now the design is to add _connect() and _disconnect() methods to TemplateInstance. Those methods iterate on the TemplateInstances Parts and call a onConnect or onDisconnect handler on them if they exist. It also propagates to any nested TemplateInstances.

Two things about this approach:

1. Each part can only have one connect or disconnect handler. This is important as each invocation of a directive has to add a handler, and it's much easier to overwrite than manage whether there was a previous handler for that directive. This becomes another reason not to compose directives yet. Making this more general will move us into hooks territory like we've discussed in issues around state.
2. This requires a tree traversal of the values tree (O(expressions in tree)). We could try to make the API signal up when a handler has been added to try to save some work...

[ruphin]

I'm somewhat concerned about point 2. I think most users don't use or need this feature, so the performance impact should be limited, or the feature should be opt-in somehow.

[simonbuchan]

I think most users don't use or need this feature

In React land, mount/unmount handlers for custom components are not unpopular? Do you mean most instances won't use or not this feature, or do you have reason to think the usage would be different?

[ruphin]

Lit-html is designed to be a DOM rendering engine, not a full framework like React. Custom Elements are the platform counterpart of React's custom components, and those have callbacks for being connected/disconnected. That API belongs in the component model layer and not the DOM rendering layer. The only reason to have a disconnect callback for Parts in lit-html is because directives may need that information, which is an implementation detail of directives. Perhaps there is a way to solve the problem there. (Or perhaps users should be discouraged from putting too much logic in directives)

…

On Sun, Feb 3, 2019, 06:28 Simon Buchan ***@***.***> wrote: I think most users don't use or need this feature In React land, mount/unmount handlers for custom components are not unpopular? Do you mean most instances won't use or not this feature, or do you have reason to think the usage would be different? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#283 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEEyr1AedCYyMPn9I7fJsYpP3ry075uks5vJnNqgaJpZM4SWJ0F> .

[simonbuchan]

Please don't take this as being an attack, I'm really not sure what your intent is here, and I want to make sure there isn't a disconnect (... ha?) between the library design and the users. Unfortunately this means that it's hard not to use combative sounding language without weasel wording everything and increasing confusion!

Lit-html is designed to be a DOM rendering engine, not a full framework like React.

This sounds like goalpost shifting or some sort of No True Scotsman. React's own description is "A JavaScript library for building user interfaces", and is commonly referred to as "The V in MVC". You can build an application with only react component state, but you really shouldn't. On the other hand, I could point at my own tiny, crappy library, which solves this particular issue by having a different interface that doesn't imply directives should have a disconnect by providing essentially a connect callback, and claim that your library is a "DOM rendering framework, not a simple library" - clearly there's differences in scope, but they are all "take some state, put DOM on screen".

In the end, there's clearly an expectation for this feature, and I don't think you're trying to argue that it shouldn't exist, but if you're under the belief that most codebases should not be using this somewhere, or that there should not be codebases using it near exclusively, then that should be made very clear, and the alternatives strongly encouraged.

For example, I've never bothered looking into using custom elements internally in a project, as they lose strong typing of the parameters, and for the cases I'd want a disconnect handler for they would be unbelievably heavyweight. On another tack, usage of observables tends to spread, much like promises do, throughout a codebase, and an app will tend to making just about every slot an observable over time. Perhaps that's not the best solution, but it's the least effort, and if you don't have a good answer, it's the one your users will use.

The only reason to have a disconnect callback for Parts in lit-html is because directives may need that information, which is an implementation detail of directives.

Most (correct) usages of react mount / unmount in react are to bind / unbind some external state to component state, which then re-renders the component with the new values. This is directly equivalent to directive connect / disconnect. My question was why you felt this not super-common, but one that will probably show up at least once, even if only in a library, in most React codebases would not show up in lit-html? The fact that they are "an implementation detail of directives" is irrelevant when when we're talking about implementing the details of directives 😉

[ruphin]

Directives are not the same as components, and they are not designed to replace components.

Using the React model, you have components with state, that react to being mounted and unmounted. In the "platform" model, Web Components are those stateful components that react to being mounted and unmounted. Lit-HTML is like the VirtualDOM engine inside React, which has nothing to do with state or anything like that, and only concerns itself with manipulating DOM nodes.

Directives are sort of a hook that allows some flexibility in how to render things, and allow patterns such as deferred rendering or other complex behaviours such as stateful subtree sorting. These directives could have some form of state, usually for caching purposes, and in those cases it may be useful to have a disconnect hook to avoid memory leaks. However, none of the builtin directives need this, and it is not a common pattern, hence my statement that it is unusual and that I expect most developers won't be using this feature.

My main point is that performance is also a feature, one which 100% of users want, and we should be careful with implementing more narrow features at the cost of performance.

[simonbuchan]

That all sounds reasonable. Are async iterables built-in? I'm pretty sure you should be calling return if present for them to clean up.

If anything I think the issue is that directives are "too powerful" and look too much like a component api. (And nearly are.)

[ruphin]

You can currently asynchronously render any kind of value that you can normally render, which includes iterables. You may run into issues when the asynchronous value resolves but the Part it is rendering to is no longer 'connected', for example because the user changed to another page or something. This can be solved by chaining the value through some 'should this still be rendered' function before it is handed off to the Part to render. It moves the solution to userspace instead of the core library, but it saves a lot of complexity in the library that is not strictly necessary.

Directives don't look like a component API to me. For example, they don't have connected/disconnected callbacks :)

[simonbuchan]

This can be solved by chaining the value through some 'should this still be rendered' function before it is handed off to the Part to render. It moves the solution to userspace instead of the core library, but it saves a lot of complexity in the library that is not strictly necessary.

This is a "you break it, you buy it" situation. The design of lit-html, in particular the render() call and needing callbacks to inject state that get fired at unpredictable from the definition site times mean it quickly becomes a lot more difficult for userspace to answer the "should I push this value" question accurately than with raw DOM calls.

Consider the trivial example of a stream with the logged in user and another with the current time. The user name is pulled out and shown in a toolbar, and when moused over, new DOM for a popout with the rest of the user details and with relative times updating using the time stream. A few seconds after a roll-out, the DOM is removed.

What does this code look like when it's correctly unsubscribing from everything correctly at the right time without support from lit-html? Your concrete advice so far has been to use custom elements, but wrapping each of those user details (of different shape and behavior) in a custom element just to correctly subscribe/unsubscribe would be a mess. On the other hand, lit-html obviously already internally has a connected/disconnected concept (otherwise it wouldn't blow up when values are pushed too late), so it would be extremely natural to just slap a mapped stream into a directive as a "show the last value out here".

Are there any non-custom component / JS-land component systems you could instead recommend? That would make the case for a DOM engine / component system split much more compelling.

For example, they don't have connected/disconnected callbacks :)

"connected" is the callback itself. Otherwise they give you pretty complete an API to hook your own logic to... except for the lack of disconnect - which by convention of observables and now React hooks would be just returning a cleanup function from the callback. All the linked issues above show that at least some of your users are viewing them this way.

[ruphin]

I think the example you mention is quite trivial to implement with lit-html. You don't need any kind of asynchronous rendering or other fancy utilities. You just have a declarative template that is rendered by lit-html, and a separate set of logic that determines what to render, and then just render the whole thing every time anything changes. This is exactly why lit-html is nice, you don't have to do any custom data monitoring and micro-updating specific locations in the DOM based on changes, you can just re-render everything every time and lit-html takes care of the complexity of determining what needs to be updated and what doesn't.

It feels to me like you're trying to use directives in ways they aren't meant to be used. All the problems you mention seem to be general application logic problems that are solvable with any sort of application architecture (component based or not) without using lit-html specific things like directives.

[simonbuchan]

Hmm, I'll have to think on that. I think it makes sense to then say, in rxjs terms, "your app view state is one big stream of lit-html instances you feed to a single render function", but I'll have to play around with both that and trying to write such cleanup without a component system but still factorable, to talk about it in more concrete terms at worst. In particular, in rx it requires a mouse out handler inside a stream operator that will remove the outer stream later, and I don't know what that would look like, but maybe it's fine.

It does feel like a big missed opportunity though, with directives being right there, and so, so close to being exactly what's needed, since lit-html is nearly tracking all the state needed already. Presuming @justinfagnani finds that directives could cleanly and efficiently support cleanup, people are going to use them that way, and that could be a very good thing if it leads to much simpler user code.

(Just to be clear, this really isn't rx specific, despite me mentioning it so much, you could trivially interop with react in the same way with a react portal directive.)

[yorrd]

well this doesn't look good 😲

[leland-kwong]

I implemented my own onConnect onDisconnect lifecycle callbacks, by dirty checking each node part with document.body.contains(part.startNode). The performance cost is pretty low, about 0.3ms for 1400 checks. In React parlance, that is equivalent to 1400 stateful components.

So far I'm pretty happy with my solution and I'm starting to think that it'd be best for us to make a separate directive for handling this. This way it'll keep the core lean.

The code I'm using is:

const partRefs = new Set()
directive(() => (part) => {
  const isConnected = partRefs.has(part)
  if (!isConnected) {
    partRefs.set(part)
    // trigger `onConnect` event
  }  
})

const render = (domNode) => {
  litHtmlRender(/* templateResult */, domNode)
  partRefs.forEach((part) => {
    const isConnected = document.body.contains(part.startNode)
    if (!isConnected) {
      partRefs.delete(part)
      // trigger `onDisconnect` event
    }
  })
}

[blikblum]

document.body.contains won't work with shadow dom

[askbeka]

weakrefs should help?

But still lit-html needs to do extra work to make this work, as far as I can see from current implementation.

And not sure yet if it is polyfillable:)

[Stradivario]

Hi there guys!

Currently simplest solution that i can work with is to use Observables with 'rxjs'

Since i am working with Decorators and i can apply some things when component is mounted and unmounted.
Decided to implement logic which collects all observables onConnectedCallback from THIS(representing Component decorated with @CustomElement() decorator) then onDisconnectedCallback i am unsubscribing from all observables.

This functionality will render template based on observable.
When the time has come and the component will be unmounted from the DOM we ensure that every Subscribed observable inside the template will unsubscribe.

Define new Map() inside the component

cls.subscriptions = new Map();

When component is mounted you could do:

cls.prototype.connectedCallback = function() {

  // Override subscribe method so we can set subscription to new Map() later when component is unmounted we can unsubscribe
  Object.keys(this).forEach(observable => {
    if (isObservable(this[observable])) {
      const original = this[observable].subscribe.bind(this[observable]);
      this[observable].subscribe = function(cb, err) {
        const subscribe = original(cb, err);
        cls.subscriptions.set(subscribe, subscribe);
        return subscribe;
      };
    }
  });
}

Then when component is unmounted

cls.prototype.disconnectedCallback = function() {
  // Disconnect from all observables when component is about to unmount
  cls.subscriptions.forEach(sub => sub.unsubscribe());
};

Decorator

import { isObservable } from 'rxjs';

interface CustomElementConfig<T> {
  extends?: string;
}

// From the TC39 Decorators proposal
interface ClassDescriptor {
  kind: 'class';
  elements: ClassElement[];
  finisher?: <T>(clazz: Constructor<T>) => undefined | Constructor<T>;
}

// From the TC39 Decorators proposal
interface ClassElement {
  kind: 'field' | 'method';
  key: PropertyKey;
  placement: 'static' | 'prototype' | 'own';
  initializer?: Function;
  extras?: ClassElement[];
  finisher?: <T>(clazz: Constructor<T>) => undefined | Constructor<T>;
  descriptor?: PropertyDescriptor;
}

type Constructor<T> = new (...args: unknown[]) => T;

const legacyCustomElement = (
  tagName: string,
  clazz: Constructor<HTMLElement>,
  options: { extends: HTMLElementTagNameMap | string }
) => {
  window.customElements.define(
    tagName,
    clazz,
    options as ElementDefinitionOptions
  );
  return clazz;
};

const standardCustomElement = (
  tagName: string,
  descriptor: ClassDescriptor,
  options: { extends: HTMLElementTagNameMap | string }
) => {
  const { kind, elements } = descriptor;
  return {
    kind,
    elements,
    // This callback is called once the class is otherwise fully defined
    finisher(clazz: Constructor<HTMLElement>) {
      window.customElements.define(
        tagName,
        clazz,
        options as ElementDefinitionOptions
      );
    }
  };
};

export const customElement = <T>(
  tag: string,
  config?: CustomElementConfig<T>
) => (classOrDescriptor: Constructor<HTMLElement> | ClassDescriptor) => {
  if (!tag || (tag && tag.indexOf('-') <= 0)) {
    throw new Error(
      `You need at least 1 dash in the custom element name! ${classOrDescriptor}`
    );
  }
  const cls = classOrDescriptor as any;
  cls.is = () => tag;
  const connectedCallback = cls.prototype.connectedCallback || function() {};
  const disconnectedCallback = cls.prototype.disconnectedCallback || function() {};

  cls.subscriptions = new Map();

  cls.prototype.disconnectedCallback = function() {
    // Disconnect from all observables when component is about to unmount
    cls.subscriptions.forEach(sub => sub.unsubscribe());
    disconnectedCallback.call(this);
  };

  cls.prototype.connectedCallback = function() {
    // Override subscribe method so we can set subscription to new Map() later when component is unmounted we can unsubscribe
    Object.keys(this).forEach(observable => {
      if (isObservable(this[observable])) {
        const original = this[observable].subscribe.bind(this[observable]);
        this[observable].subscribe = function(cb, err) {
          const subscribe = original(cb, err);
          cls.subscriptions.set(subscribe, subscribe);
          return subscribe;
        };
      }
    });
    connectedCallback.call(this);
  };

  if (typeof cls === 'function') {
    legacyCustomElement(tag, cls, { extends: config.extends });
  } else {
    standardCustomElement(tag, cls, { extends: config.extends });
  }
};

Lit async Directive

import { directive, Part } from '../lit-html/lit-html';
import { Subscribable } from 'rxjs';

type SubscribableOrPromiseLike<T> = Subscribable<T> | PromiseLike<T>;

interface PreviousValue<T> {
  readonly value: T;
  readonly subscribableOrPromiseLike: SubscribableOrPromiseLike<T>;
}

// For each part, remember the value that was last rendered to the part by the
// subscribe directive, and the subscribable that was last set as a value.
// The subscribable is used as a unique key to check if the last value
// rendered to the part was with subscribe. If not, we'll always re-render the
// value passed to subscribe.
const previousValues = new WeakMap<Part, PreviousValue<unknown>>();

/**
 * A directive that renders the items of a subscribable, replacing
 * previous values with new values, so that only one value is ever rendered
 * at a time.
 *
 * @param value A async
 */
export const async = directive(
  <T>(subscribableOrPromiseLike: SubscribableOrPromiseLike<T>) => (
    part: Part
  ) => {
    // If subscribableOrPromiseLike is neither a subscribable or
    // a promise like, throw an error
    if (
      !('then' in subscribableOrPromiseLike) &&
      !('subscribe' in subscribableOrPromiseLike)
    ) {
      throw new Error(
        'subscribableOrPromiseLike must be a subscribable or a promise like'
      );
    }
 
    // If we have already set up this subscribable in this part, we
    // don't need to do anything
    const previousValue = previousValues.get(part);

    if (
      previousValue !== undefined &&
      subscribableOrPromiseLike === previousValue.subscribableOrPromiseLike
    ) {
      return;
    }

    const cb = (value: T) => {
      // If we have the same value and the same subscribable in the same part,
      // we don't need to do anything
      if (
        previousValue !== undefined &&
        part.value === previousValue.value &&
        subscribableOrPromiseLike === previousValue.subscribableOrPromiseLike
      ) {
        return;
      }

      part.setValue(value);
      part.commit();
      previousValues.set(part, { value, subscribableOrPromiseLike });
    };

    if ('then' in subscribableOrPromiseLike) {
      subscribableOrPromiseLike.then(cb);
      return;
    }
    subscribableOrPromiseLike.subscribe(cb);
  }
);

Usage

import { html } from 'lit-html';
import { LitElement } from 'lit-element';
import { customElement } from './custome-element';
import { timer } from 'rxjs';

@customElement('app-component')
export class AppComponent extends LitElement {
   private timer = timer(1, 1000).pipe(map(v => v));
   render() {
       return html`<h1>App Component: Seconds from start ${async(this.timer)}</h1>`;
   }
}

[simonbuchan]

@Stradivario while this might be a nice surface API for wrapping rendering, as far as I can tell, this isn't really using rxjs to solve the issue, it's using HTML custom elements (in an internally complicated way) so you get a disconnection callback, which is the missing piece. This is already the recommendation of the lit-html team.

This issue is about being able to use alternatives systems, either component systems, or just value streams directly (Observable or not) that can integrate with lit-html in a lightweight way, without it having to also track what is already rendered in the DOM, which lit-html is sometimes already doing for it's own purposes.

[Stradivario]

This issue is about being able to use alternatives systems, either component systems, or just value streams directly (Observable or not) that can integrate with lit-html in a lightweight way, without it having to also track what is already rendered in the DOM, which lit-html is sometimes already doing for it's own purposes.

You are correct! Totally agree with you. One thing which is coming to my mind is that if for some reason 'lit-html' decide to re-render the whole template instead of single part of it will lead to subscribing many times to the same observable. Even the disconnected callback will not help that way...

Watching this discussion with interest!

Regards!

@simonbuchan

[prasannavl]

@Stradivario - I had written this ages ago - https://github.com/prasannavl/icomponent - You might want to take a look at this - basically has everything you wrote there, and a bunch more, but in a more minimalist and generic way.

@justinfagnani - Any progress on the connect, disconnect aspects you had mentioned? The last I evaluated lit-html for a project, the lack of disconnect was one of my biggest blockers for self managed list items before going out of scope, animating themselves, etc. Has there been any work in this area?

[Stradivario]

@Stradivario - I had written this ages ago - https://github.com/prasannavl/icomponent - You might want to take a look at this - basically has everything you wrote there, and a bunch more, but in a more minimalist and generic way.

@justinfagnani - Any progress on the connect, disconnect aspects you had mentioned? The last I evaluated lit-html for a project, the lack of disconnect was one of my biggest blockers for self managed list items before going out of scope, animating themselves, etc. Has there been any work in this area?

Thank you very much i am checking what is going on there!

[Legioth]

I took some time to test the approach that I described in #283 (comment). Everything seems to work and the result is in https://gist.github.com/Legioth/2594a6043f54e391615cefea73a5a079.

I realized that it's not enough to rely on lit-html itself since we also need to know when the render root gets connected or disconnected. In the case of LitElement, this can be handled using lifecycle callbacks as I've done in the StatefulLitElement helper class. Other cases could also be handled manually from application logic by manually running the activate and deactive functions as appropriate.

I had to use the private __parts property in TemplateInstance to find the current child parts and override NodePart.prototype.commit to find out which child parts are used before and after doing the actual commit. Everything else can be implemented based on public API.

I haven't run any benchmarks, but it seems like the performance overhead of this approach should be quite minimal. For parts that don't need the tracking, the overhead is just a single boolean check to see if the part is currently active. For parts that are tracked, I'm doing simple O(n) iterations over all child parts to detect changes and a tree traversal when some part is activated or deactivated. Based on this, I would hope that this approach or at least some enablers could be integrated into the core library.

[jancellor]

I'm also coming from the angle of "what's missing?" when trying to use just lit-html (no custom components etc). This is not directly about adding a disconnect handler for directives but rather how/where to get similar functionality.

The ~70 lines linked below uses a class LitView with a render function and a directive to commit itself using that render function which also looks for child LitViews within the result of the render function and calls mounted/updated/unmounted on those children. Also by keeping a reference to the part, we can do an update (re-render) on the subtree. This is very simple and seems to work but I'm not sure how naive it is.

https://pastebin.com/8HATKpc1

Perhaps this isn't even even necessary. Is it really required to automatically detect the disconnect of a component by its absence in a parent's render-function's template? Is manually calling connected/disconnected from the parent difficult, error-prone or at odds with the concept of declarative views? I haven't yet found a suggestion of how to actually use lit-html in more than a toy example that just calls render from a setInterval.

[justinfagnani]

@jancellor we're working on a disconnect handler for lit-html 2.0.

But to be clear, lit-html is not intended to have it's own component system. It's designed to be use with custom elements, both to instantiate other custom elements via plain tag names in the templates, and to be used as the template system for a custom element class like LitElement.

Yes, directives can keep state, so you can abuse them to build a component system, but it's not something we're very interested in supporting.

[jancellor]

@justinfagnani, acknowledged. And thanks for explicitly stating it's designed to be used with custom elements. I'm still interested it how/if it can be used in a non-web-component component model (which as in my example above doesn't necessarily mean storing state in directives) but here may not be the best place to ask.

[pbastowski]

@justinfagnani If you want to have a look at how lit-html could be used to create web apps without using lit-element then here is a CodeSandbox that I have put together to test the idea.

Also, the library haunted implements a virtual component model on top of lit-html that does not require web components. It is worth looking at.

[PaulMaly]

@justinfagnani Actually, we should have this feature not because we use lit-html for creating a component system, but because if a directive can be applied to the DOM element it definitely should have we a way to know that this element is not accessible anymore because lit-html removes it from the DOM. A good example is any async operations in directives. We can perform them on element render, but can't abort if the element removed. It's very common but at the same time very important thing.

[kevinpschaaf]

This feature has been implemented in the lit-next branch via #1432, slated for the next major release of lit-html.

[Stradivario]

Thank you very much for the effort creating this PR!

Much appreciated! @kevinpschaaf

@jancellor

I have managed to create a whole ui-kit with components using lit-html https://github.com/rxdi/ui-kit
I have managed to create production ready boilerplate also with lit html https://github.com/rxdi/starter-client-side-lit-html
Even without disconnectedCallback for directives it is up to the developers how they handle subscription of parts.

I really don't see any problems working with WebComponents and LitHtml in production grade applications which for sure are not "toys" if you insist to show you an example please contact me at my email.

Regards,
Kristiyan Tachev

[electrovir]

FYI the DisconnectableDirective which was created when resolving this issue was renamed to AsyncDirective (in #1533). (It took me a while to track that down.)