Add stricter types to mixinBehaviors

[mlcui-corp]

Context: Chromium is adding TypeScript support for their Polymer code. As there are still some legacy behaviors that need to be used until they can be ported to a mixin, mixinBehaviors needs to be used in the meantime - but due to the incomplete types of the function, all calls of mixinBehaviors need to be manually typed.

This PR fixes this problem by providing a much stricter type than the current one, merging the behaviors specified in the arguments as well as applying the same mixins that LegacyElementMixin does. This requires each behavior passed to be typed correctly, and (unfortunately) exposes these properties to the class:

• Polymer-related properties like 'properties', 'observers' 'listeners'. Using Omit<T> to try to get around this results in type errors as Omit<T> rewrites the whole object type, resulting in TypeScript converting all instance member functions in the type to instance member properties, causing TS2425 when trying to override/implement a behavior function in a class.
• "Private" properties which shouldn't be used by consumers of the behavior. This is a limitation of TypeScript as there is no notion of a "private property" on an object.

Both of these can be worked around by manually specifying a public interface for the behavior instead.

Note that the helper type used in one of the overloads requires the use of TypeScript 4.1, released on November 2020. The overload is only necessary if more than 3 types are used, and can be removed if necessary (but IMO most people should be using TypeScript 4.1 by now).

[mlcui-corp]

@kevinpschaaf @sorvell Friendly ping.

[kevinpschaaf]

This LGTM, asking @justinfagnani to give a quick TS expert review.

[kevinpschaaf]

Note that the helper type used in one of the overloads requires the use of TypeScript 4.1, released on November 2020. The overload is only necessary if more than 3 types are used, and can be removed if necessary (but IMO most people should be using TypeScript 4.1 by now).

@mlcui-google This looks fine, but the requirement to use TS 4.1 would potentially be a breaking change for existing users, since we shipped 3.x before it was released. Could you remove the variable-length behaviors overload (and feel free to add as many overloads as you need to satisfy your use cases)?

Thanks!

[mlcui-corp]

Sure. The longest I've seen is [5 behaviors](https://source.chromium.org/chromium/infra/infra/+/main:go/src/infra/appengine/sheriff-o-matic/frontend/elements/som-alert-view/som-alert-view.js?q=pcre:yes%20mixinBehaviors%5Cs\*%5C\(%5Cs\*%5C%5B%5Cs\*%5Cw%2B%5Cs\*,%5Cs\*%5Cw%2B%5Cs\*,%5Cs\*%5Cw%2B%5Cs\*,%5Cs\*%5Cw%2B%5Cs\*,%5Cs\*%5Cw%2B%5Cs*), so I've updated this PR with overloads to 5 behaviors (and add a fallback for 6+ behaviors).

[mlcui-corp]

@justinfagnani Friendly ping.

[mlcui-corp]

An update on this - this doesn't work when calling mixinBehaviors with an array of behaviors. Let's not merge this in until this is resolved.

[mlcui-corp]

Yeah, I don't think this is possible without using some kind of recursive type helper, and in older TypeScript versions it's almost impossible to iterate through a tuple without a very hacky workaround.

On the other hand, trying to force the overloads to not be applied when a nested array occurs is tricky as there is no way to apply a type constraint that "T is not an array" in a generic definition, which is needed for the overloads to work. I don't think this is feasible, unless @justinfagnani can think of a better way of making this work.

[mlcui-corp]

Figured it out - have overloads for the broken cases before the working cases. This should now work (and has been tested with Chromium's Polymer code).

I've also removed the LegacyElementMixin type definitions in mixinBehaviors - the side effect of applying LegacyElementMixin in mixinBehaviors (IMO) shouldn't be relied on users, and users can manually apply LegacyElementMixin to the class before calling mixinBehaviors if they explicitly want the old API. It also simplifies type errors occurring as a result of mixinBehaviors significantly.

edit: Turns out that rest tuple types were introduced in TypeScript 3.0, which is after the current version of TypeScript used in this repo (2.9.1). May need to rethink how to manage these tricky tuple types.

[mlcui-corp]

This has gotten into a rabbit-hole of TypeScript types. I don't believe that overload resolution works as expected with type inference, so there's no clean way of getting around this unfortunately.

I'm closing this for now as it's very complicated for something which is a minor improvement (inferring the correct type, which is usually overridden using as) with many edge cases which would cause more confusion than not having this PR.