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....
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.
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);
});
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.
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.
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.
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 togetValueResolver(), which wraps it into an observable-ish of arrays of resolved values (empty, string, node or template).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.
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:
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.
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?
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 notifications@github.com 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
https://github.com/Polymer/lit-html/issues/283#issuecomment-460024541,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEEyr1AedCYyMPn9I7fJsYpP3ry075uks5vJnNqgaJpZM4SWJ0F
.
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 😉
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.
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.)
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 :)
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.
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.
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.)
@ruphin just for me to understand because this is a very pressing issue for us: so what you're saying is that instead of using an rxjs subject in a single directive, rather trigger the complete render cycle anew when one reactive fires?
First argument for unmount handlers: inplace pipes. Code gets SO MUCH MORE READABLE (sorry for yelling) when you do certain piping inline. <input type="checkbox" ?checked=${someSubject.map(v => !!x)} /> avoids having to store another variable with that resulting observable. I've done some counting, in the test code base we maintain right now where we test rxjs with lit-html we're using this roughly 10 times per web component
Second argument: If I got your comment right, that would mean that our render function would now have to have > 50 parameters with the values of the rxjs subjects we're using in any given template - that's at least how many we're using at the same time in many applications. Of course, you could hide those behind a state object or something, but that's much more boilerplate and "framework logic" people have to understand as opposed to just defining a few subjects, importing another few (because they're globals) and then going ahead and rendering the template result. Also, you could split off modules / web components / components (which obviously everybody does anyways) to limit the numbers of subjects you need for one render call, but that would mean you'd have to still stop all those subscriptions when disconnecting your component. Alternatively, you'd have to register every subject somehow into the render cycle for them to automatically be stopped - which is a new requirement for the developer to understand which we want to avoid.
tl;dr: pretty much any alternative to unmount handlers seem like more boilerplate and / or framework logic to me, no matter how you implement it. We're trying to stay as close to pure TypeScript / the browser API with as few new paradigms as possible. I realize that everything we need can be done with application structure, but does it need to be?
lit-html offers us a great option to stay as close to TypeScript (aka what people know already) without introducing new stuff people have to know about. That's the reason we want to write a new base class based on lit+rxjs instead of lit-element.
I don't quite understand. Your example input element does not use any directives. It works now and it doesn't need this feature to work.
That looks like it was assuming native observable support. Without that it would actually look more like ?checked=${stream(someSubject.map(x => !!x))}, where stream(value$) is the observable directive equivalent to the (obsolete) directive I posted in the initial description.
I suppose you could go the whole way on supporting custom template argument types so people could hook in direct observable, or react, or whatever, support themselves, not sure what the value is.
Having looked into this a little further, it's totally possible to write using just one top-level render without using directives, or a component system. You just end up with something like redux, or the one big stream style I mentioned. Here's a rxjs example, since it's easier to talk about.
Notice that you will repeatedly need rx.combineLatest(a$, b$).pipe(ops.map(([a, b]) => ... html... ${a} ... ${b} ...)), and that adding and removing the popup while unsubscribing the timer requires a cond$.pipe(ops.switchMap(cond => cond ? true$ : false$)). These are not too scary when you get used to rxjs, but it's pretty unfamiliar initially, and it is busying up the actual view structure with stream operators to get at the underlying value. On the other hand, using directives, you get this much simpler code, but without a way to unsubscribe the subscriptions continue to get leaked (check the preview console repeating the current time the number of times you have hovered over the blue div). If lit-html natively supported the observable proposal (which is not rxjs specific), then it would feel even more natural, but that's a minor improvement.
To be fair, it does make a lot of sense to make sure performance won't bomb when doing this, but I don't remember anything obvious with my earlier spike on this - but at least at the time it looked like it would have a lot of impact on the internal architecture in order to keep the argument information around in a way that you know when to call the cleanups later - perhaps its better now?
The other thing to keep in mind is that maybe render() should now return a cleanup function, which would be equivalent to render(null, parent), calling any existing cleanups, but not needing to mess with the DOM?
@simonbuchan thanks for the clarification of my sloppy code.
@ruphin what I meant was more like <input type="checkbox" ?checked=${stream(someSubject.pipe(map(v => !!x)))} />, as for the other arguments --> what @simonbuchan said :D well put
Could this be addressed either in user space or in the library itself by using ideas similar to the good old reactive variable system from Meteor?
The basic idea is that you add a listener on any value used by an update. Schedule a new update when any of those listeners are notified. Before doing the next update, unregister all old listeners so that we'll only listen for updates on values touched by the next update. This requires that you keep track of all listeners registered for a given update, i.e. through a global array (with some additional tweaks if you need to support reentrant updates).
This would allow using a simple ?checked=${stream(something)} pattern in the template but it would require wrapping some logic around your html calls to unregister old listeners and set up the global for collecting new ones. If using LitElement, this could quite naturally be integrated around the render() function.
@Legioth then you would restart all subscriptions every time any of them fires a new value? That seems like it defies the idea of having rxjs subjects inline, couldn't you then just as well merge(...) all of them and pass their arguments to the render function? That would bring us back to the above mentioned points. Sorry if I misunderstood you.
For subscriptions, the same scheme can be used with a minor tweak. Collect all used subscriptions, and cancel any that were used by the previous update but not the current one. Your "only" problem is to define equality for the subscriptions to know which one should be retained. This is something that you'd have to deal with in any case to avoid subscribing over an over again every time the directive is run.
On the library level, this approach could be an efficient way of implementing the disconnect handlers. Any time render() is run for a given root, record all parts for which at least one "disconnectable" directive had been run. After the render, go through all parts that were missing from the last render of the same root and notify them. In that way, the overhead when not using any "disconnectable" directives is limited to checking whether each directive is "disconnectable".
One interesting question is whether it would make more sense on the library level to have "persistent" directives that are also run in different ways depending on whether it's the first time that part is rendered.
Actually, directive equality seems like it would be something you would want even without disconnect. @ruphin mentioned earlier that stateful subtree sorting is something that should be in scope for directives - I assume that means things like Reacts keyed lists? How would that work if it couldn't tell what the previous value was?
One approach is to reverse the directive factory function: instead of directive((...args) => part => { ... }), use directive2(part => (...args) => { ... }), where the outer callback is called once on "mount", and the inner callback is called immediately after, and for each update where the factory function is the same.
With some change-detecting directive like:
const listDirective = directive2(function listFactory(part) {
let oldItems;
return function listCallback(newItems) {
if (!oldItems) {
// apply initial value...
} else {
const added = newItems.filter(item => !oldItems.includes(item));
const removed = oldItems.filter(item => !newItems.includes(item));
// order...
// apply updates...
}
oldItems = newItems;
};
});
function listView(items) {
return html`<ul>${listDirective(items)}`;
}
render(listView(a), document.body);
render(listView(b), document.body);
For arguments sake, say directive2(factory)(...args) would return an object like { factory, args }, then here the first call to render(), gets from listDirective(a) an object like { factory: listFactory, args: [a] }, it then calls listFactory(part) to get listCallback, then stashes something like { factory: listFactory, update: listCallback }, then calls listCallback(a) to actually update. When it re-renders with the result of listDirective(b), it sees that factory in the directive result and the stashed state is the same, and just calls the stashed listCallback with b.
If that makes sense, then you could also support something like:
const listDirective = directive3(part => {
/* setup, state here as in directive2 */
return {
apply(...args) { ... },
cleanup() { ... },
};
});
which doesn't seem like it would add much further overhead from directive2, in terms of state to track.
@ruphin does the directive2 setup make sense to you, for those stateful, but non-cleaning up directives you were mentioning before?
As you correctly deduced, directives already have access to some form of state, since this is necessary for some functions like keyed lists. Since the directive is called with the Part it is rendering to, it can use that Part as a key to store state.
const partValues = new WeakMap();
const detectChange = directive(value => part => {
if (value !== partValue(part)) {
console.log("Value changed")
}
partValues.set(part, value);
})
Or you could use the ham-fisted solution of storing state directly into the Part itself (Don't do this)
const detectChange = directive(value => part => {
if (value !== part.__value) {
console.log("Value changed")
}
part.__value = value;
})
As you can imagine you can use this to call factory methods or whatever initial setup you like when a directive is "mounted". Directives are called each time they are rendered, so at render time you can execute whatever code you like. The difficulty here is exactly that, directives are only being called when they are rendered. If at any point another value is rendered in this Part, the directive will not know about it. Although you could do something extremely dirty like overriding the setValue() method on the Part, that would still only catch when another value is rendered into the Part, and not when the Part is no longer included in the render tree. The only solution to this is adding some kind of lifecycle hooks to Parts, which would effectively mean that the render complexity changes to O(n) where n is every Part that was ever previously rendered, instead of only the Parts that are being rendered.
I've been considering this issue quite a lot lately and I maintain my opinion that lifecycle callbacks for Parts are not in scope. The purpose of lit-html is to provide a function that turns some kind of state into DOM. By adding lifecycle callbacks or hooks to Parts they effectively become components, and providing a component model was explicitly stated as out of scope for lit-html. There are plenty of solutions that provide component models, such as Web Components, that lit-html can be paired with.
I know that Web Components may not offer a satisfactory solution for solutions that depend on reactive programming, but perhaps that is an indication that the rendering solution you need is not a declarative map of state to DOM, which is what lit-html is, but a rendering solution that comes with a component model, such as React.
Again, this is just my opinion. I'm not an official maintainer of this library, so I don't have any authoritative voice on what will happen. It is up to Justin to decide what gets included and what doesn't. I just want to make sure that lit-html keeps with it's core values of being small, efficient, and focused.
@ruphin Sure, I just don't want half a feature! Do it so that it supports what people are going to use it for or not at all.
@justinfagnani I don't suppose you've been following all our blather? Is there some glaring issue with supporting my directive2 API above (other than the name and backwards compat stupidness), and thus directive3? I imagine largely that it would be a lot churn for it to work that way.
The difference to what it sounds like you're designing, the directive registering a cleanup handler, being that directives can then keep their state local, which could make them more efficient at updating, but more importantly, you know immediately if there is a cleanup handler to call or not, it won't ever change, and there can only be one, possibly making it easier to only allocate a template cleanup handler / register for cleanup immediately on render, not every commit? Not sure if that makes a difference.
When you say O(parts in tree), do you mean O(parts in removed subtrees)? Would O(parts in removed templates where some part has cleanup) be sufficiently better?
I believe it's possible to implement all of this in user space by wrapping render() to manage window.partsFromLastRender and window.partsFromThisRender.
const renderPartsCache = new WeakMap();
function renderWithState(result, container, options) {
window.partsFromLastRender = renderPartsCache.get(container) || new Map();
window.partsFromThisRender = new Map();
render(result, container, options);
renderPartsCache.set(container, window.partsFromThisRender);
// Unmount for parts that were no longer used
window.partsFromLastRender.forEach((unmountCallbacks, part) => {
if (!window.partsFromThisRender.has(part)) {
unmountCallbacks.forEach((callback) => callback(part));
}
});
delete window.partsFromLastRender;
delete window.partsFromThisRender;
}
function statefulDirective(updateCallback, mountCallback, unmountCallback) {
return directive(part => {
// Mount if part was not previously used
if (mountCallback && !window.partsFromLastRender.has(part)) {
mountCallback(part);
}
updateCallback(part);
// Record state for this part, including unmount callback if present
if (mountCallback || unmountCallback) {
const statefulDirectives = window.partsFromThisRender.get(part) || [];
if (unmountCallback) statefulDirectives.push(unmountCallback);
window.partsFromThisRender.set(part, statefulDirectives);
}
});
}
To support reentrant calls to renderWithState, it would be necessary to store previous values of the two global variables and then restoring the previous values (in a finally block) instead of just overwriting and deleting.
The actual API for statefulDirective can be varied to e.g. reverse the operations as suggested for directive2. I'm only focusing on how data is propagated between different pieces here. With my suggested design, statefulDirective(foo) is actually equivalent of directive(foo) since neither of the ifs are evaluated.
@Legioth this is a great start, now I know what you mean. Excuse me for being slow. One issue remains if I have read correctly: you define stuff on the window object. However, that assumes that there is only one render function which will be called. If there is more than one and execution alternates (different components), all directives will always be stopped. You could solve this by defining the maps on the component, no big thing. You would additionally have to utilize the respective component lifecycle callbacks to clean up the unneeded parts after the very last render, which is possible. tl;dr: nice :+1:
Let me try to put this into our test code base and check if I overlooked anything.
@simonbuchan what do you think, do you see any trouble with this? You seem to be working with rxjs as well :) if we can do it in user land, let's do it in user land.
The state must be put in window or some other global location since the directive doesn't know for which container it's being run. This is one aspect where the library could offer a little help e.g. by passing container and/or options to directives.
I still believe the best approach is as I suggested in the text but didn't bother to write out in the actual code: renderWithState must keep track of previous values and restore them before returning. That should work since the execution cannot to my understanding alternate back and forth arbitrarily but rather traverses the tree in hierarchical order (or completely independently with LitElement that uses microtask scheduling for the actual rendering).
@Legioth I cooked that up into a test, which works pretty OK (I changed the statefulDirective API to match my directive3 above, as it's far easier to clean up). It's hard to tell what the performance impact is, as the UI update cost is completely dominated in this test by NodePart#_commitText() and browser layout. Not sure how you would isolate that out - updating a style attribute? A property?
But I'm a little concerned in the theoretical sense: imagine in some extreme case you have a million subscribed streams (the DOM probably would have fallen over at this point, but go with me), with one item updating every frame - with this userspace fix you have a O(stateful parts) re-render cost, while a native lit-html update could pay essentially nothing over before, as it can see that no directives have updated in the updated subtrees (at the same cost as it currently pays to see if any part arguments have changed) and the only cost is the actual DOM update under part.setValue(). If there were a template getting continuously replaced every frame (e.g. flickering a popup for some effect - presumably cache is being used to avoid the DOM cost), that itself contained a stateful part, lit-html only needs to look inside the mounting and unmounting template for stateful parts to call mount and unmount on.
It's a neat trick, and it seems that DOM is so awfully slow, and rxjs not that much better, that it's unlikely to ever show up in profiling, but it seems to be in opposition to lit-html's placement as being low-overhead.
@Legioth Unfortunately I just tested the case of nesting stateful directives and templates, and that breaks it - when the outer stream fires, it updates the inner template with the new value, but it's not inside a stateful render and thus there are no last / this frame parts. The stack looks like:
at eval (stateful-render.ts:25)
at NodePart.commit (parts.ts:197)
at TemplateInstance.update (template-instance.ts:55)
at NodePart._commitTemplateResult (parts.ts:267)
at NodePart.commit (parts.ts:207)
at updatePart (repeat.ts:39)
at eval (repeat.ts:443)
at NodePart.commit (parts.ts:197)
at TemplateInstance.update (template-instance.ts:55)
at NodePart._commitTemplateResult (parts.ts:267)
And eval (stateful-render.ts) is the only user code, so I don't think this can be worked around 😢
@simonbuchan Your directive2 API is already supported, just with a different implementation:
const previousItems = new WeakMap();
const listDirective = directive(items => part => {
let oldItems = previousItems.get(part);
if (!oldItems) {
// apply initial value...
} else {
const added = items.filter(item => !oldItems.includes(item));
const removed = items.filter(item => !newItems.includes(item));
// order...
// apply updates...
}
previousItems.set(part, items);
});
As I mentioned, detecting when a directive is first executed in a specific Part, or what it's previous value was on that Part is all possible in the current implementation. The only difficulty is detecting when it is no longer rendered in a future render.
@Legioth That is an excellent suggestion. It would be easy to wrap lit-html and export a render and directive function that implement this. Instead of a global variable on window they can just use a closure to share this state.
When you use this, it will break when you use stateful directives outside a stateful render. Fortunately lit-html does never call render itself, so as long as you control where you enter lit-html and do not pass templates with stateful directives to non-stateful render functions, it should work just fine. Can you share a reproduction of the exception you are running into?
Your
directive2API is already supported,
"supported" is a strong word for that! 😉 Not sure if there's a perf. cost for doing that, but that seems like it would work (assuming lit-html doesn't later decide to create a new part instance each directive call, rather than keeping it around)
But it wasn't really the point - the point was that if lit-html could support that without additional cost, then it would naturally extend to directive3.
When you use this, it will break when you use stateful directives outside a stateful render. Fortunately lit-html does never call
renderitself, so as long as you control where you enter lit-html and do not pass templates with stateful directives to non-stateful render functions, it should work just fine. Can you share a reproduction of the exception you are running into?
I posted the full stack, the exception is just the partsFromLastFrame.get() fails with partsFromLastFrame being undefined, as we are not inside any render() - we're inside a directive updating a template.
You should be able to run, debug and edit from that link, if you want to play around with it.
Now I see. My time to apologize for being slow. The problem here is that render() is not the only thing that causes directives to run. Directives are also run when some other directive (e.g. <div>${until(somePromise, "Initial text")}</div>) asynchronously triggers commit() on any part. This would be fine for e.g. AttributePart because it only performs yet another incremental update for an existing and retained part. The problem is NodePart which can add or remove elements and through that also their corresponding parts and directives.
This means is that we'd have to wrap NodePart.commit() instead of wrapping the top-level render() function. I haven't verified this, but I assume that render() always starts from NodePart which means that render() itself wouldn't need to be separately wrapped. The "best" way of wrapping NodePart.commit() would be to use a subclass of NodePart. This is, however, not practical because new instances are created in several different locations. The second best option is the ugly approach of updating NodePart.prototype.commit.
Excuse me for jumping in here, but I've become very interested in solving this problem as well. Lit-html solves the declarative rendering problem quite nicely, and while I don't want to see it expand into a full component solution (which I see some debate on here), I believe that directives such as asyncReplace are critical to making the library useful in non-trivial projects.
I still have a lot to catch up on, reading through this thread and the proposed design solutions, but in the meantime I had to find a workaround for a prototype I've been building. For the short term, I've specifically had to solve the unmounting issue with asyncReplace. Thus, I've temporarily added the following code to that particular directive:
for await (let v of value) {
// Check to make sure that value is the still the current value of
// the part, and if not bail because a new value owns this part
if (part.value !== value) {
break
}
+ if (!part.startNode.isConnected) {
+ value.return()
+ break
+ }
This does the trick, but is of course extremely specific to this directive. I can't imagine that the solution in this case is so simple, so I'm hoping somebody could point out any problems that I'm overlooking.
Edit, this can be simplified to:
+ if (!part.startNode.isConnected) {
- value.return()
+ break
+ }
@davezuko for ... of already sends .return() when it leaves the loop for iterables, are you perhaps using an iterator? (I'd also expect that to work, though)
In my testing, async*() does in fact seem to work correctly when unmounted, I'm not sure why, given that code.
Thanks for the reply, @simonbuchan. If you take a look at this example (taken from the lit-html docs), you'll notice that return() is not called when the directive is unmounted, which happens after 5 seconds. It only returns if you use the patched version of asyncReplace mentioned above. Is there something I'm overlooking?
Note: The explicit return() call is not needed, as you point out. However, the loop does need to exit, which it does not currently. Here's a fork of the docs example where asyncReplace has been patched: https://codepen.io/davezuko/pen/NoYBzx.
Yeah, that example behavior is what I would expect, I think I must have just triggered the part.value test before. async*() could easily be extended to also include the part.startNode.isConnected test, but I suspect that would probably break under the cache() directive, which disconnects and reconnects existing DOM. Probably not a huge issue, in the case where you're using both, repeatedly leaking is probably worse than UI failing to update, as in the latter case, you can see and fix it quickly? YMMV.
random remark: another use case for this are intersection observers :)
Quick and dirty writeup:
export const whenSeen = directive((cb: () => void) => (part: NodePart) => {
const subject = new Subject();
const node = part.startNode.parentElement!;
// differentiate between ios and non-ios. the polyfill sucks so we write our own
if (window.IntersectionObserver) {
const intersectionObserver = new IntersectionObserver(
x => {
if (x[0].isIntersecting) {
subject.next();
}
},
{ rootMargin: '0px' },
);
intersectionObserver.observe(node);
} else {
// TODO polyfill
}
subject.pipe(debounceTime(30)).subscribe(cb);
// TODO unsubscribe as soon as we have disconnect handlers
// intersectionObserver.unobserve(part.startNode.parentElement!);
// intersectionObserver.disconnect();
});
One issue remains if I have read correctly: you define stuff on the window object. However, that assumes that there is only one render function which will be called. If there is more than one and execution alternates (different components), all directives will always be stopped
Is possible to pass a shared state between each render call through options. It will be available in part.options. If you are using lit-element part.options.eventContext points to the element instance. This can be used as a temporary solution to unsubscribe used observables.
There's the issue for dynamic show directives that will only be notified when owner element is disconnected even if is hidden in template.
There's also a potential issue when calling same template many times e.g. iterating array items using .map. In this case the part instance is reused even with different data.
All in all, i'm with @ruphin. lit-html should stay small, focused and efficient. If there's a efficient way to handle disconnect in directives, good, but if it would sacrifice performance better live without it and rely on higher level abstractions like custom element connect/disconnectCallback
just wondering: what's the official way to proceed with async-replace? It should have the same issue. I'd like to borrow from a potential official way of doing things in the end. Or will async-replace just be removed?
I do agree that keeping lit-html small is a good idea, however, we just cannot ignore the fact that there are streams out there that will create memory leaks with all current implementations. The only way to work around the issue atm is to stop the subscriptions when some kind of component is unloaded (web component?), but that's insufficient because there might be subscriptions nested within the template of a web component that has a different lifecycle. Yes, it has its own lifecycle, even if it's not a very complex one. I still don't see how to deal with this without unmount-handlers.
@justinfagnani are you still working on an implementation or is this still being discussed internally? A commitment on this question would really help us, even if the actual implementation is not going to happen for a few weeks :)
well this doesn't look good :astonished:
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
}
})
}
document.body.contains won't work with shadow dom
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:)
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());
};
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 });
}
};
async Directiveimport { 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);
}
);
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>`;
}
}
@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.
This issue is about being able to use alternatives systems, either component systems, or just value streams directly (
Observableor not) that can integrate withlit-htmlin 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
@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 - 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,disconnectaspects you had mentioned? The last I evaluatedlit-htmlfor a project, the lack ofdisconnectwas 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!
I took some time to test the approach that I described in https://github.com/Polymer/lit-html/issues/283#issuecomment-461708287. 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.
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.
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.
@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.
@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.
@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.
@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.
This feature has been implemented in the lit-next branch via https://github.com/Polymer/lit-html/pull/1432, slated for the next major release of lit-html.
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
Most helpful comment
This feature has been implemented in the
lit-nextbranch via https://github.com/Polymer/lit-html/pull/1432, slated for the next major release of lit-html.