Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to override default converter and default getter/setter #891

Closed
bschlenk opened this issue Feb 6, 2020 · 4 comments
Closed

Comments

@bschlenk
Copy link
Contributor

bschlenk commented Feb 6, 2020

Description

It would be great if it were possible to override the defaultConverter for attributes, and the default getter/setter for properties.

My team is converting our existing web components library over to lit-element. We made some not-so-great decisions in our components, but we want to keep the behavior for now for backwards compatibility reasons. The main bad decision is that we special case boolean attributes so that the string "false" is treated falsy (contrary to native boolean attributes which are a presence check). We also coerce properties, so setting a boolean property to the string "false" would convert it to boolean false. We have similar conversions for number, array, and object.

With the way lit is now, this means that every single boolean property requires us to set a custom converter, and we also need to define our own getter/setter. This is a lot of boiler plate for us, and really puts a damper on the development process. If we were able to override the defaultConverter & getters/setters, then we could simply subclass LitElement, override these to get our behavior, and that would be it.

@justinfagnani
Copy link
Contributor

You can already override static createProperty() already to provide a different default property declaration, which can in turn use a different default property converter.

Overriding the accessor creation is harder right now because state _ensureClassProperties() and static _classProperties are private. We could potentially make them protected, or offer another static just for defining the property.

@justinfagnani justinfagnani self-assigned this Feb 6, 2020
@bschlenk
Copy link
Contributor Author

I tried overriding the createProperty() function but I couldn't get it to work. I basically copied the one you linked above into another class and modified the get and set in the Object.defineProperty call, but it didn't work. I don't remember what sort of errors I was getting though. I can try it again when I have time so I can give you some more detailed error messages. One issue with copying it is that it has to reference the private _ensureClassProperties and _classProperties. I'm also not all that sure about how polymorphism works with static methods on classes in js.

We've come up with a workaround for now, it's a function that we pass each class into that reads the existing static properties and for each one that is set to type Boolean, we do our own Object.defineProperty to create a getter and setter. We just need to remember to pass all our classes into this function before we call customElements.define.

@javier-garcia-meteologica

Typescript complains when you override createProperty with a similarly specified function.

[tsserver 2341] [E] Property '_ensureClassProperties' is private and only accessible within class 'UpdatingElement'.
[tsserver 2341] [E] Property '_classProperties' is private and only accessible within class 'UpdatingElement'.
[tsserver 2341] [E] Property '_requestUpdate' is private and only accessible within class 'UpdatingElement'.

@sorvell
Copy link
Member

sorvell commented Mar 3, 2020

Closing this issue in favor of #911 where we're tracking this issue.

@sorvell sorvell closed this as completed Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants