Discussion: supported serializers out of the box

[43081j]

I recently wrote this blog post about the fact that we don't support type: Object and type: Array like Polymer did. I also made this repository.

This is a thing people assume quite often, even in the recent docs PR.

So I think we need to make a decision before 1.0, one of the following two choices:

• Ship serializers for Object and Array (and possibly require that consumers import them rather than supporting them by default)
• Somehow make it clearer that type is not a type constructor but rather a type serializer (at this point we can't really rename type but maybe we could document it very well)

I'm happy to PR this if we go with the former.

Right now, people almost always assume because they see examples use type: Number, type: String, etc, they should then use type: Date, type: Object, type: Array and so on. This won't work as expected, but people rarely bump into this realisation as they pass their data via properties rather than attributes.

We do want to keep the core as light as we can, so if we do introduce this, it probably belongs in an extra module we must import ourselves.

cc @justinfagnani

[justinfagnani]

cc @sorvell and @kevinpschaaf

What about offering a JSON type?

[43081j]

FYI that's pretty much what I did in my example repo. I think it's a good idea instead of detecting usage of Object.

Reason being, it'll make it more obvious that it's a serializer, not a type constructor. As you have to explicitly import it and pass it through.

[pietmichal]

I'd really go with renaming type to something else.
Everyone's initial thought is different than anticipated.
And it confuses both Polymer and non-Polymer folks apparently.

You can use JSON.parse for fromAttribute and JSON.stringify for toAttribute to reflect object and array values properly.

I prefer LitElement taking a serialization/deserialization path which makes the intention clear without any magic surrounding it.

[43081j]

If we weren't so close to 1.0, i would agree.

It depends on how breaking you feel like being just before 1.0 i suppose.

The correct solution to me seems to be to rename type to serializer or some such thing. But, if we want to maintain similarity with Polymer and previous lit versions, that isn't an option.

If you, @UnrealProgrammer , have a look at the repo i threw up in OP, that implements exactly what you described. a JSON serializer pretty much, which works for both Array and Object.

At the very least, we should avoid implementing support for Object and Array, as it would lead to people assuming all other type constructors also work.

[adamdoyle]

It might be useful if the documentation better explained more about the expected form of the "function used for both serialization and deserialization" (see quote below), to prevent the misunderstandings mentioned about the type field.

From readme.md:

The value can be a function used for both serialization and deserialization, or it can be an object with individual functions via the optional keys, fromAttribute and toAttribute. type defaults to the String constructor, and so does the toAttribute and fromAttribute keys.

So my understanding is that type accepts something that (1) effectively implements interface AttributeSerializer<T = any> { fromAttribute(v: string): T; toAttribute(v: T): string|null; }, as well as (2) "a function used for both serialization and deserialization".

When I see examples with type: String / type: Number / etc. and consider the fact that neither String nor Number have toAttribute / fromAttribute methods, I assume that they must be the former, (1). But I still don't really understand what that means. The Number constructor doesn't take a string and return a number. Are those just hard-coded special cases?

edit: I'm wrong, Number does accept a String and return a number, e.g. let x = Number('3'); Also I see this in the code: type AttributeType<T = any> = AttributeSerializer<T>|((value: string) => T);, which definitely explains the documentation. But I still think the documentation could be clearer.

[43081j]

I think the concept as a whole is a little too confusing going forward. It isn't an easy thing to document and should be a lot simpler really.

We could introduce a set of default serializers and make the docs reflect that, or maybe at least ensure we no longer use type: Object and such in the docs anywhere.

[sorvell]

Marking this as high priority here since the resolution may change API.

What exists now is definitely not ideal. What we refer to externally as type is internally called an AttributeSerializer. Users can write their own AttributeSerializer's with toAttribute and fromAttribute methods.

However, there's also a private _propertyValueToAttribute which specially handles type: Boolean. We'll likely need to add more special casing here as well for other corner cases.

Previously we had a special BooleanAttribute symbol that needed to be used for boolean behavior, but users objected to this as weird. There was a strong desire to just use standard types.

I think perhaps we could leave type but disambiguate it from the AttributeSerializer by perhaps letting users specify that via serializer. This would be the least change that might also help enhance clarity.

[43081j]

If you want some help, i'm happy to do a PR.

We could support type of common primitives like polymer did to avoid confusion.

Then have serializer like you say, which is what we have now.