DO NOT MERGE: replace symbols with dumb classes

[web-padawan]

Do not merge. This is a PR for discussion purposes only.
The wip/next branch has to be updated accordingly as we decide here.

Overview

The root problem is microsoft/TypeScript#17744 that is still unsolved.
See also "Related TS issues" below for more context.

These are 3 approaches that we might use for mixins that need to have protected methods, that can be overridden by the subclass including the super call.

1. Symbol approach

• Not using protected methods in the mixins

• Using Symbol as recommended by Elix instead

Pros

• Methods are truly protected (can't be called from outside, e.g. by 3rd party code)

• Methods are "public" in terms of TS and can be annotated on the interface

• Explicit exports helping to make sure the consumers have compilation errors in case the new version was released an the internal API contracts of the protected methods have changed

• Methods are excluded from the DevTools console's auto-complete list (good for user)

• Symbols also avoid potential name conflicts if a component user wants to extend a custom element with their own methods.

Cons

• Consumers have to import symbols if they need to override protected methods

• Jump to definition is not ideal (jumps to the place where Symbol() is created)

2. Dumb class approach

• Using protected methods in the mixins

• Using abstract class SlottedItemsClass extends LitElement to "annotate" them

Pros

• No need to import anything (except the mixin itself) for the consumer component

• Methods are easy to access in the DevTools console (good for us as maintainers)

Cons

• Both interface (for public API) and abstract class (for protected API) are needed

• The abstract class ends up in the JS output (as empty class with no methods)

• The mixin function needs to confusingly accept the argument with a type cast (instead of the actual expected class) in order to pick up the annotated protected methods from it:

// with dumb class: confusing
<T extends Constructor<SlottedItemsClass>>(base: T)

// without dumb class: clean
<T extends Constructor<LitElement>>(base: T)

• Need for extra super check: super._focus && super._focus() - this is needed because the abstract classes mark methods using _focus?(), i.e. "possibly undefined"

• Methods can be only called in tests with as any workaround

• Jump to definition from the subclass goes to a dumb class

3. Public methods approach

• Keep methods public, add them on the interface

• Jump to definition works as expected

Pros

• No extra code needed (symbols / dumb classes)

Cons

• TS users would get protected methods listed in the code completion

• Messing up public and protected API might require tweaks for tools

Summary

Personally, I like symbols approach more because it's cleaner, and it's not about "fighting with how TypeScrip works". But I'm ok with using the protected methods, too.

Related TS issues

• Generating type definitions for mixin classes with protected members microsoft/TypeScript#17744 (comment)

• [ts] 'private' modifier cannot appear on a type member. microsoft/TypeScript#25163 (comment)

• Unexpected TS4094 with the build parameter declaration: true microsoft/TypeScript#17293 (comment)

[web-padawan]

Agreed to use dumb classes for now. Closing in favor of vaadin/component-mixins#7