Change setValue method in FormControlMixin to limit TS visibility

[electrostaticfleece]

Problem

The FormControlMixin currently exposes the setValue method as a public method on elements that use the mixin. When using the FormControlMixin to build a published component for other teams to use, this can lead to confusion when consuming teams use TypeScript. Since setValue is public it appears as though it is intended for direct use by said consuming teams.

Impact

This misunderstanding may cause improper usage patterns and bypasses the intended abstractions we've designed for managing state and interactions within form control elements.

Proposed Solution

Changing setValue from a public method to a protected method would be ideal on our end. However, I understand this might not be ideal on your end. We're open to other potential solutions as well.

[michaelwarren1106]

Thanks for the issue and discussion!

I don't really think there is a great solution here unfortunately. JS doesn't natively have protected methods or properties. protected would only actually help in Typescript applications, but wouldn't prevent folks from being able to call setValue() at runtime. But, private doesn't work well either, because if the method is JS native private (like instance.#setValue() then the implementing component that uses FormControlMixin can't call it either.

I'm inclined to think that the best case is for consumers to just make sure not to add setValue() to their documentation so that it won't show up as public even though technically it kinda has to be.

[MaartenPetiet]

Hi, I would like to add to this request. In my project it would really help us if the properties that are meant to be protected, are also indicated as protected properties.

Our use case is that we generate a custom-elements.json that generates our Storybook documentation and VS Code autocomplete. Now that these properties are not marked as protected, they get wrongfully added, which gives the impression that they are in fact public. That they are not truly private in compiled JS is less of an issue for us.

Thanks!

[calebdwilliams]

I can look into this but it is currently my belief that there is no way to indicate a mixin field/method as protected in TypeScript with proper intellisense support. If you have an example of that I’d be happy to take a look at this.

[MaartenPetiet]

@calebdwilliams thanks for your quick response. When I mark a property as protected and try to use it from outside my class, I get the following error in VS Code:

Next to intellisense, we also generate a custom-elements.json manifest. I have created a small example, where the card class has two properties. The propertyMarkedProtected is declared as protected, which adds "privacy": "protected" to the custom-elements.json manifest. This can be used by tools like Storybook to generate interface documentation, where members that are protected are ignored. You can find the example here: https://codesandbox.io/p/devbox/d6nxlf?file=%2Fcustom-elements-manifest.config.js

[calebdwilliams]

What I'm getting at is that you can't define a property or field as protected from the context of a mixin and have TypeScript recognize it.

That's why we provide a reference to the public API of the mixin as part of the generic argument to the mixin. Alternatively you can't provide protected metadata to an interface or I would have. It's different for base classes, but because we allow you to bring your own base class we don't have the same sort of flexibility that something like Lit does based off of TypeScript's design constraints.

I would love to solve for this, but I'm not sure TypeScript will allow us to, but if you can get a working example for a technique, I'd be happy to do a refactor.

[MaartenPetiet]

@calebdwilliams ah I see what you mean. I did not know this was a limitation of the current TypeScript functionality. I researched a bit but I see there are still open issues for this on the TypeScript GitHub page. Thanks for clarifying!