Conversation
| @trait( | ||
| selector: ":is(map)" | ||
| ) |
There was a problem hiding this comment.
Should we also allow this on member shapes targeting map shapes?
There was a problem hiding this comment.
That makes sense and I'll add the selector. Should we also allow this for Document shapes?
There was a problem hiding this comment.
Ah now that is a good question. It's tricky because document doesn't necessarily indicate an Object.. Do we currently have support for preserving ordering in documents in smithy4s? Or use it in the dynamic module? I think if we don't already have it, then let's leave support for it out for now and we can always add it later if we decide to
There was a problem hiding this comment.
I added a comment to the PR that implements this in smithy4s https://github.com/disneystreaming/smithy4s/pull/1820/files#diff-723646fa221b5ad4799cce9b4e34f0eb3411e577fecfd88f5ce96b58bffdbb5bR508. The SchemaVisitorJCodec does use the preserveMapOrder boolean when handling document objects.
There was a problem hiding this comment.
Okay interesting, I guess that makes sense from the perspective of a user using the smithy4s-json module. In that case, maybe we should allow this trait on Document and it will only have applicability in the case of objects.. @kubukoz thoughts?
There was a problem hiding this comment.
Can't see anything problematic with it at the moment...
let's just make sure it's supported in the document codecs as well. Encode-side too, right? ok I see that in the docs of the trait, makes sense to me
kubukoz
left a comment
kubukoz
left a comment
There was a problem hiding this comment.
suggestion: compliance tests for this?
|
Mentioned this earlier, but so we don't forget, we'll need to add the new trait here: |
|
Aside: I guess we can technically add support for this trait on smithy4s 0.18 as well and deprecate the current methods for "preserveSortOrder" we should discuss on discord perhaps |
3 participants
Adding a new trait that is applicable to maps that should denote that the keys should be serialized/deserialized in order.