Skip to content

Rename JSValueConstructible and JSValueConvertible #87

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

Closed
MaxDesiatov opened this issue Sep 29, 2020 · 7 comments · Fixed by #88
Closed

Rename JSValueConstructible and JSValueConvertible #87

MaxDesiatov opened this issue Sep 29, 2020 · 7 comments · Fixed by #88
Labels
enhancement New feature or request

Comments

@MaxDesiatov
Copy link
Contributor

I always forget which is which, maybe something like ConvertibleToJSValue and ConstructibleFromJSValue would be more explicit?

@MaxDesiatov MaxDesiatov added the enhancement New feature or request label Sep 29, 2020
@j-f1
Copy link
Member

j-f1 commented Sep 29, 2020

The fork I based those implementations off of used JSValueEncodable and JSValueDecodable. I kept the existing names because they match existing type names (like CustomStringConvertible). Maybe JSValueConstructible could be renamed ExpressibleByJSValue to match ExpressibleByStringLiteral?

@MaxDesiatov
Copy link
Contributor Author

I like ExpressibleByJSValue 👍

@kateinoigakukun
Copy link
Member

kateinoigakukun commented Sep 30, 2020

I prefer ConstructibleFromJSValue because ExpressibleByXXX is mainly used for literal expressible types in stdlib.

They are examples of protocol naming uses in stdlib.

  • XXXConvertible: Self -> Target
  • XXXBrigable: Self -> Target & Target -> Self
  • ExpressibleByXXX: Literal -> Self

@MaxDesiatov
Copy link
Contributor Author

I also wanted to mention that Expressible is used mainly for literals. But then I recalled that Swift Argument Parser library from Apple folks has ExpressibleByArgument protocol, which is not strictly about literals. Either way, I don't have a strong opinion here about alternatives, I just find the status quo a bit confusing 😅

@kateinoigakukun
Copy link
Member

I discussed this topic in the Japanese Swift community and I think ConstructibleFromJSValue and ConvertibleToJSValue are the best pair.

  • JSValueConvertible/JSValueConstructible

This pair is really ambiguous about "input" and "output".

  • JSValueEncodable/JSValueDecodable

It feels like Encode/Decode implies that member methods do some serialization. But our protocol does not serialize but just convert JS/Swift values.

  • JSValueConvertible/ExpressibleByJSValue

From stdlib use cases, ExpressibleBy prefix implies that the protocol is related to literal.

  • JSValueConvertible/ConstructibleFromJSValue

XxxConvertible is used to represent one-way transformation in stdlib, and data info may be lost while transformation.
If the transformation is intended to preserve data info, the protocol has explicit "Lossless" prefix. (e.g. LosslessStringConvertible)

  • ConvertibleToJSValue/ConstructibleFromJSValue

It feels like this pair is the most explicit representation and is precise pair.

@j-f1
Copy link
Member

j-f1 commented Oct 1, 2020

Created #88. One last thing to consider: is JSValueCodable a good name? Should we rename it or maybe even delete it?

public typealias JSValueCodable = ConvertibleToJSValue & ConstructibleFromJSValue

@MaxDesiatov
Copy link
Contributor Author

I just started preparing a PR like that myself, but you were first 😄

I'd suggest JSValueCompatible as a replacement for JSValueCodable.

@j-f1 j-f1 closed this as completed in #88 Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants