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

Moves ValueWithSerializer back into public API #605

Closed

Conversation

Daeda88
Copy link
Contributor

@Daeda88 Daeda88 commented Aug 29, 2024

It had become part of commons-internal which kind of defeats the purpose.

Also made FirebaseEncoder/FirebaseDecoder an interface in the public API. Useful for writing custom Serializers that have custom behaviour on Firebase

Daeda88 and others added 2 commits August 29, 2024 10:39
It had become part of commons-internal which kind of defeats the purpose.

Also made FirebaseEncoder/FirebaseDecoder an interface in the public API. Useful for writing custom Serializers that have custom behaviour on Firebase
@nbransby
Copy link
Member

It had become part of commons-internal which kind of defeats the purpose.

I'm going to need more of an explanation, is it to fix this issue #590 ?

@Daeda88
Copy link
Contributor Author

Daeda88 commented Aug 29, 2024

No I actually ran into it myself. The ValueWithSerializer class was originally in the regular commons library and allows encoding a value when dealing with File paths. But it's no longer accessible so I moved it back. Though I'm not entirely sure if this was introduced by Splendo or GitLive. Technically you can achieve the same behavior with a custom SerializerModule though, this does add some convenience

@nbransby
Copy link
Member

allows encoding a value when dealing with File paths

Could you give a code example of what you mean? A search of ValueWithSerializer in the README doesn't return any results.

@mr-kew
Copy link
Contributor

mr-kew commented Aug 30, 2024

@Daeda88 Does this somehow relate to #604?

@Daeda88
Copy link
Contributor Author

Daeda88 commented Aug 30, 2024

Its mostly about:

public data class DocumentReference {
    public suspend inline fun update(vararg fieldsAndValues: Pair<String, Any?>, buildSettings: EncodeSettings.Builder.() -> Unit = {})
}

where the Any in the pair is encoded using dev.gitlive.firebase.firestore.encode(it, buildSettings), which means you cannot set a custom serializer for it.

But I see it was introduced by Splendo in #448 and if that is so, I actually think we should solve it a different way more consistent with the rest of the library: by adding support for passing the serializer to the update methods. I'll change this PR to reflect that and ping you when it's done.

@Daeda88
Copy link
Contributor Author

Daeda88 commented Aug 30, 2024

Dropping in favour of #607

@Daeda88 Daeda88 closed this Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants