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

Change jsonKeyEncodingStrategy and jsonKeyDecodingStrategy to non-optional with default values #108

Open
brendanlensink opened this issue Dec 14, 2022 · 1 comment

Comments

@brendanlensink
Copy link
Collaborator

As they currently exist, you can create a request like

struct UserRequest: Request {
    // ...

    var jsonKeyDecodingStrategy: JSONDecoder.KeyDecodingStrategy { return .convertFromSnakeCase }
}

that looks correct but won't work, since it should be JSONDecoder.KeyDecodingStrategy?.

To fix this, and make it more difficult to mess up, I think we should switch both encoding strategy params to non-optional and just provide a default setting, like we do with smartUnwrapKey

@amyoulton amyoulton assigned amyoulton and unassigned amyoulton Dec 14, 2022
@amyoulton
Copy link
Contributor

Findings:

Currently we have two places that we can set encoding/decoding, the request itself or the netable implementation configurations. Currently, we're checking if there is a strategy within the individual request existing and if it doesn't exist, it defaults to the config one, which is set to use default keys.

Since we removed the optional, this would never happen, which means that config option is rendered useless.

We need to think on how we want to solve this issue as to not affect the current structure of Config being the base decoding/encoding strategy which is overridden by a requests if it exists! My gut tells me that we are going to want to keep it optional because this structure currently makes a lot of sense, but then I'm unsure how to handle that error we encountered.

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

No branches or pull requests

2 participants