Add nothing sentinel value

[justinfagnani]

Rendering nothing to a Part will cause it to clear. Useful to make sure that elements with ShadowRoots can trigger <slot></slot> fallback content:

import {html, nothing} from 'lit-html';

html`<my-element>${condition ? 'something' : nothing}</my-element>`

[ruphin]

I was always fond of this solution. I'll take a look at implementing this

[csvn]

I like it, might make it much clearer than using '' to indicate that nothing should be rendered.

Just a thought about naming. Might empty be a better name? It'll match the :empty CSS selector, as well as saving two keystrokes 😉

I also sometimes see important constants being uppercased (e.g. EMPTY/NOTHING) in javascript, is that common or helpful for distinguishing between local variables and imported constants?

[web-padawan]

Thumbs up for nothing as it sounds reasonable. Regarding the empty, it sounds like adjective, not a noun (although, it can be probably used as a noun, too).

[csvn]

@web-padawan Good point. Might have made less sense than I thought. At least I didn't propose a verb 😄

[ruphin]

The nice thing about nothing is the readability combined with render.

render(nothing, targetElement)

[43081j]

this is pretty much the null object pattern, i'd disagree with it being called nothing though.

usually with this pattern, you'd call it NullSomething, such as NullPart

[ruphin]

Note that there are two different sentinels that are "null"-like with different purposes.

• noChange - which signals the renderer to make no changes to the current content and effectively do nothing
• nothing - which signals the renderer to actively clear the current rendered content and render nothing.

These naming schemes are more descriptive than null, and they combine well with the render verb: render(nothing, somewhere), render(noChange, somewhere).

[43081j]

The name looks like psuedo-code to me, I still think there are better, more technical names that are used in other languages and libraries.

Ideally IMO we should just have a null part. If we want to render nothing, we can then pass that static part to avoid creating a new part every time and to support any kind of cache behaviour. This is a pretty common pattern across all languages, as it saves you having to do null checks.

Wanting to avoid rendering (noChange) sounds like a separate feature request to me.

[ruphin]

Passing new values should not create new parts (with the exception of nested templates or lists). There already exists a part for each dynamic value in a template, and passing new values directs these parts to render something. You don't have to create a new part to render a new value, and with the current implementation of lit-html there is no use for something like a static part.

The nothing value is a static sentinel that signals parts to clear their content, which is pretty much exactly what you suggest.

noChange is not a feature request, it is already supported.

[stramel]

Couldn't we just use null and use that to trigger the clear? Seems like that would be better DX?

[43081j]

@ruphin Of course, it is to render "nothing", i.e. a null part. The equivalent of render(null, somewhere) if we supported that.

I suggest we go with NullPart, if anything, or something similar rather than a plain english name.

[ruphin]

It is reasonable to use null instead of a custom sentinel object.

My main concern with it is that null will have different behaviour in NodePart and AttributePart:

render(html`<div attr=${ null }>${ null }</div>`, document.body)
// <div attr="null"></div>

Having a distinct sentinel for this case makes the interaction more explicit, and avoids confusion. (Why does null remove this thing but not that thing?)

---

On the other hand, using null simplifies the implementation, and it has a little resonance with how innerHTML handles null:

document.getElementById('container').innerHTML = null;
// <div id="container"></div>

---

At this moment null and undefined values in NodeParts render as '', so there is already some dissimilarity in how NodePart and AttributePart handle null. We could change this behaviour to clear instead of rendering '', but I suspect this will cause a slowdown in general use cases, as it will unnecessarily destroy and recreate text nodes when toggling between primitives and undefined / null. We could leave the current behaviour for undefined, and change null to clear instead, but the more you think about it the more it seems arbitrary and vague.

I will make a PR that implements nothing as originally suggested, and we can discuss the merit of different options based on that.

[ruphin]

Note:

document.getElementById('container').innerHTML = null;
// <div id="container"></div>

document.getElementById('container').innerHTML = undefined;
// <div id="container">undefined</div>

[43081j]

Adding null checks will of course introduce overhead too, but @stramel has a point that it is easier to understand / more useable.

At large scale it would be a noticeable performance difference i imagine and could introduce the need for more null checks further down the stack.

Anyhow the naming isn't great, lets see what the opinions of others are. Plain english and code don't look too good together IMO.

[ruphin]

We already have an explicit check for null, so using null as a marker will not add any additional null checks whatsoever. The only performance impact is caused by inadvertently removing and creating TextNodes when switching between rendering null and other primitives, where the current implementation just changes the TextNode content.

Adding a sentinel like nothing has almost no impact to performance, since it will be the last value we test for in setValue, and almost all common rendered values will trigger before hitting that point in the chain. It does add more additional bytes to the total payload.

---

There is plenty precedent in the public API of this library where we use English language: removeNodes, noChange, createMarker, render. I don't see why this is suddenly a problem.

[43081j]

Where do we already check for null parts (or template results)? Out of interest.

Introducing the null object pattern generally results in simpler implementation but also becomes useful for removing the need for null checks which definitely do have a performance overhead. If we already need such checks, though, sure, it makes little difference but those are generally the two reasons for such a pattern.

nothing is a bit of an unconventional name for such a thing. Your examples all describe what they are, nothing describes nothing at all, it isn't clear what it is or why it even exists out of context. Context-dependant names aren't a great thing, it deserves a more descriptive name.

I'll stick with my suggestion for now, NullPart. After all, it is just a suggestion :)

[justinfagnani]

One phrasing I'd like to clear up is that we don't have "null Parts". Parts exist, usually, as a result of expressions, and value flow into them. So we just have null values, and with this proposal a nothing value.

I like nothing because it's clear. The DOM handles null and undefined in inconsistent ways. We could try to "fix" the DOM and normalize that behavior, but it'd be our opinion against the specs and implementations.

[43081j]

It holds no descriptive value, has no meaning even in context.

render(nothing)

What is nothing? Is it our variable, an imported one, a part, a template, it isn't clear at all. I really don't think we should be introducing names into the API which are so non-descriptive, it makes code a lot more difficult to follow. It looks simpler.. as a word, but it has very little meaning.

It would be nice if there was a slightly more descriptive name that could be agreed on.

edit:

part of this may even be related to casing. Although Foo implies it is a constructor, at least it becomes clear it isn't some random variable we've thrown in there.. that or more descriptiveness would help.

[ruphin]

We check for null when setting values that are primitives (strings, numbers, null, undefined, etc. Basically things that aren't objects or functions) because we render '' instead of null and undefined.

[ruphin]

As mentioned in the issue, this feature is only to solve a very specific issue regarding fallback content in slots. Most users will never see or use nothing, and can continue using whatever they are using now, which can be null or '' or whatever they like. For the users that actually need it, it will be descriptive enough.

[43081j]

I don't think usage amounts justify non-descriptive APIs and naming.

It should be slightly more performant than using a new empty template each time, too.

I wouldn't like to debate a name into such depth anyway so if justin would like it to still be called nothing, fair enough. I would prefer a more descriptive name but an opinion is an opinion :)

[ruphin]

What do you mean with a new empty template? Maybe I missed a use case that you have for this feature.

[43081j]

When combined with lit-element, for example:

if (foo) {
  return html``;
}

new template result per render, its something i've already seen people doing quite often. perf benefits would be achieved by using the null obj pattern and having a static empty result (which is essentially the same thing but instantiated once and used by ref).

same can be applied to things like ${condition ? foo : html``}

[motss]

Just wondering. What happens if something like cache(condition ? 'something' : nothing)is written?

[ruphin]

The cache(condition ? 'something' : nothing) statement is a bit strange. The sole purpose of the cache directive is to cache template instances, but none of the possible arguments are templates, so it does absolutely nothing, and it is functionally identical to condition ? 'something' : nothing except it performs some extra useless checks.

I will assume you meant something like this:

cache(condition ? html`something` : nothing)

The short answer is that it works as expected. If the condition is false, it renders nothing. If the condition is true, it renders the template. If the condition switches several times, the template instance is cached.

The cache directive does not care what kind of argument it gets. If the argument changed since the previous render, and the previous argument was a template, it "saves" what was rendered previously into a cache. Then it renders the new argument, which can be another directive, or a builtin like nothing, or whatever lit-html normally renders. If it gets a template that it currently holds in the cache, it will use that cached instance to render instead of creating a new instance for that template.

[motss]

It basically clears my doubts. Thank you for the clear explanation. 👍

…

On Thu, Jan 17, 2019, 3:31 AM Goffert van Gool ***@***.***> wrote: The cache(condition ? 'something' : nothing) statement is a bit strange. The sole purpose of the cache directive is to cache template instances, but none of the possible arguments are templates, so it does absolutely nothing, and it is functionally identical to condition ? 'something' : nothing except it performs some extra useless checks. I will assume you meant something like this: cache(condition ? html`something` : nothing) The short answer is that it works as expected. If the condition is false, it renders nothing. If the condition is true, it renders the template. If the condition switches several times, the template instance is cached. The cache directive does not care what kind of argument it gets. If the argument changed since the previous render, and the previous argument was a template, it "saves" what was rendered previously into a cache. Then it renders the new argument, which can be another directive, or a builtin like nothing, or whatever lit-html normally renders. If it gets a template that it currently holds in the cache, it will use that cached instance to render instead of creating a new instance for that template. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#652 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKHcj8N9bmIp6CYWA6gvj3tSGbyA9c7Lks5vD34FgaJpZM4YtzsC> .