Skip to content

URIEncoder does not correctly encode primitive collections #292

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
bfrearson opened this issue Sep 24, 2023 · 7 comments
Closed

URIEncoder does not correctly encode primitive collections #292

bfrearson opened this issue Sep 24, 2023 · 7 comments
Labels
status/triage Collecting information required to triage the issue.

Comments

@bfrearson
Copy link
Contributor

Trying to encode a primitive collection (e.g. an array of Strings) from a Codable type fails because the URIEncoder rejects nested object types.

It would be beneficial to support this, as there we may have query items or forms which contain multiple values for the same key. I believe URIDecoder already does support collecting identical keys into a collection.

@czechboy0 czechboy0 added the status/triage Collecting information required to triage the issue. label Sep 25, 2023
@czechboy0
Copy link
Contributor

@bfrearson can you provide an example of the Codable type?

Which one doesn't work, and what's the error?

typealias Foo = [String]

or

struct Foo: Codable {
  var bar: [String]
}

I'd expect the former to work, but possibly not the latter.

@czechboy0
Copy link
Contributor

Ping @bfrearson, when you have a moment, I'm wondering if you remember which case from the above was failing for you, thanks!

@bfrearson
Copy link
Contributor Author

Hey, sorry for the delay - been super slammed at the moment!

It would be the second option that failed:

struct Foo: Codable {
  var bar: [String]
}

@czechboy0
Copy link
Contributor

Yes, that's expected. Nested containers are not supported, only top level containers. So the array has to be the top level item, and the name bar should be the name of the parameter instead.

@czechboy0
Copy link
Contributor

It's also possible you're trying to do #259?

@bfrearson
Copy link
Contributor Author

bfrearson commented Oct 26, 2023

Yes it looks as if this covers the same issue.

Tbh this was an abstract thought I had while working on the encoder, rather than a real world barrier that I hit. I don't think any of the APIs I'm currently using personally need to support this, but I think there should definitely be support for handling them.

@czechboy0
Copy link
Contributor

Understood, thank you. Let me close this issue then and reference it from #259, as it already covers this then.

@czechboy0 czechboy0 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/triage Collecting information required to triage the issue.
Projects
None yet
Development

No branches or pull requests

2 participants