Lit-element: _render?

[developit]

What's the reason for using the leading _ there?

[notwaldorf]

We have a convention that protected methods start with a _, and public methods don't.

[notwaldorf]

Also https://github.com/PolymerLabs/lit-element/issues/23

[hfjallemark]

My initial reaction is that it feels kind of hacky and like a workaround when you prefix names like that, something you would only do internally in your code if needed — not in the public api of your library that your users will have to see. If it’s only a naming convention the Polymer team uses for protected methods I would recommend not polluting lit-element’s public api with that, it is definitely not a well-established naming convention that everyone wants to use.

[eskan]

feel the same override a protected method is weird.

[Lodin]

@eskan, why? Protected method is designed to be overridden.

[Lodin]

@hfjallemark I would agree with you if we had the same situation as we have in React. Placing properties into separate containers is a quite good idea and it resolves situation with name conflicts. But web components are different. We define internal methods and public API properties side-by-side, so we have to distinguish them from each other. And I suppose that underscore is a good way to do it. It may change in the future when the private fields proposal are adopted it will change though.

[eskan]

@lodin when i write an element i implement constructor, properties, connectedcallback and _render. It's just unusual to me as a polymer user but i'm not aware about JS convention and best practice.

[hfjallemark]

@Lodin but is it really realistic that the 6 lifecycle methods in lit-element would ever collide with somebody's public api properties? Enough to warrant the underscores?

Personally, as a React developer I love the idea of lit-element -- being able to build web components the way I build React components. But that naming convention is really off-putting, makes it feel like it's a work-in-progress.

[notwaldorf]

@hfjallemark To be fair, if you see the issue that I linked (#23) it was exactly raised by a react developer, about how in some cases lit-element didn’t work with react because of the name collision

[TimvdLippe]

I think the difference here is the terminology of "the user". We have two types of users here: 1. the user of lit-element to define a webcomponent and 2. the user of the webcomponent itself.

For type 1, having render makes the most sense. However, for type 2 this does not. Type 2 expects all public API to be callable as is, and the render function is not intended to be called by type 2, it is intended to be overridden by type 1 as an implementation detail. Therefore, if we would call it render, type 1 expects it to be public API. It is not (it is protected, intended to be overridden), thus _render was chosen.

[Lodin]

@hfjallemark Yes, it is realistic. I just faced it myself as I described at #23. I am a React developer too and I just tried to implement Render Props approach, but due to same name of render method I met a naming conflict and was forced to use $render property instead. I think using underscores is a less evil than to force my component users feel sick because of strange property names.

Also underscore itself is not a strange thing. Time to time this kind of convention appears in JS and other languages. In Dart, for example, it is (or at least it was when I was experimenting with it) convention for private variables. Since there are not only public and private, but also protected properties, I'm fully supporting Polymer naming approach where public does not have any leading underscore, protected has one and private has two. It allows to separate concerns and avoid naming collision, considering that there can be many other properties due to class inheritance.

[hfjallemark]

@Lodin @notwaldorf agree, that is a valid use case (at least considering you can't do "real" render props lit-element like you can in React).

I also know that some languages use underscore by convention (e.g. C# does it for private fields), still doesn't make it less horrible 😄

I think @Lodin's first suggestion with the Callback suffix is better and aligns with the native web components terminology (and could make adoption easier). Especially considering the audience -- JavaScript developers, maybe even primarily developers with React (or similar) experience. Protected methods doesn't exist in JavaScript, it's something you would see in fully fledged OOP languages like Java or C# and for many it's a new, unnecessary concept that has to be explained to them, increasing the entry barrier.

E.g. nowhere in React's documentation do they mention protected methods, although they are (at least as much as the lit-element's lifecycle methods). Instead of linking to Wikipedia about protected visibility in OOP they simply say:

Each component has several “lifecycle methods” that you can override to run code at particular times in the process. Methods prefixed with will are called right before something happens, and methods prefixed with did are called right after something happens.

MDN also doesn't mention protected methods:

Using the lifecycle callbacks

You can define several different callbacks inside a custom element's constructor, which fire at different points in the element's lifecycle:

connectedCallback: Invoked when the custom element is first connected to the document's DOM.
disconnectedCallback: Invoked when the custom element is disconnected from the document's DOM.
adoptedCallback: Invoked when the custom element is moved to a new document.
attributeChangedCallback: Invoked when one of the custom element's attributes is added, removed, or changed.

Sorry for slightly derailing the original discussion, but I'm very excited about lit-element and would really love to see the api and "first-time encounter experience" to get a bit more polished (and reducing cognitive load/reasons for asking questions).

[merlinnot]

Callback suffix feels natural when it comes to a lifecycle if a web component. I like the idea.

[Lodin]

@hfjallemark I have a thought about why React doesn't have protected mehtods. It's because in fact they are not. In lit-element render is called from inside the component itself, from its prototype chain, so there is a reason to make them protected. For React, AFAIK, things are different. React components by default are stubs, which are processed by outer system that is hidden inside ReactDOM.render() method. And all lifecycle methods are called by this system from component's instance. That's why they are not protected by definition. Well, I might be wrong, and would appreciate if you correct me.

[hfjallemark]

@Lodin you are probably right about how React calls those functions, but to me it is an implementation detail whether they get called externally or from a base class. JavaScript still does not have protected visibility in its language, so it is incorrect to call it that -- it is a public method that happens to be called by its base class and if you really want to compare it to something that exists in an OOP language (although I don't see the point) I would argue it closer resembles an abstract method.

The more important question I think is; as a user of this api, what purpose does it serve to have know all of this? Can't we just call them lifecycle methods and try to stay as close to regular web component terminology as possible (e.g. using a Callback suffix)?

[Westbrook]

It would seem that naming when working with a non-synthetic base class (web components) is less an implementation detail than a contract detail. When using web components, you've contracted with the browser that certain realities will be true about the environment that your code will run in, an environment that in and of itself needs to have room to shift, get better, and expand as more and more people use it. This being so, naming methods close to the platform's lifecycle methods can be danger to the long term health of the platform. I understand the cognitive benefits of using a method like renderCallback, you've already got connectedCallback, et al, so that definitely makes a lot of sense and in a completely synthetic context like React, I'd be right there with you. However, what if the platform decides it wants to get into the render game? With some template instantiation work that's going on, it might not be as far fetched as it might sound. Then the spec goes to TC39, there's a lot of excitement about the broader benefits of the addition, and then, boom, SmooshGate 2020...

Maybe it's just a nomenclatural issue, protected method _doesn't really_ mean anything in JS, sure, so maybe the right word in a new word? A word that means "can be added to native objects with minimal danger" but also "expected to exist on the extended object for use and reuse as it is then later extended with that same level of security". I'm up for suggestions.

[sorvell]

Thanks for all of the feedback on this issue. Right now we feel there's not a slam dunk right answer on this, but we do appreciate that the _render is not entirely satisfying. We're still weighing the options here.

[sorvell]

We've now decided to remove the leading _ for all protected API. So you just write render like this:

render() {
  return html`<div>${this.foo}</div>`;
}

This was done based on feedback and to match the convention used in the platform for things like connectedCallback.

[TimPietrusky]

Thank you super much, I was super confused why suddenly it's called render in the lit-element documentation when the examples of https://polymer.github.io/pwa-starter-kit/configuring-and-personalizing/#naming-and-code-conventions are still telling me it's _render. But I guess those need an update.