Ignore default on refinements #795
Conversation
In this PR, I attempt to solve #791 which renders product member as if they were required. The issue happens because the Smithy definitions happen in a v1 IDL language version and the shape in question gets `Default` trait added. The boolean that indicates whether or not the member should be wrapped in option check for `Required` or `Default` trait on the shape, and if they are there, it will make it required. So a non-required field, gets an automatic `Default` because it is defined in a v1 spec, and thus the rendering code scraps the optionality off of it. In this PR, I replace the `Default` trait presence check with a more involved one. The check is now: `if hasDefault && typeIsNotExternal`. I assume a Type.external necessarily mean a refined type (but I'm not 100% sure about that. In any case, this change behaves how I want and we can see that in the few examples that are updated in this PR. I added and extra field with `@required` to make sure it was still respected.
|
This comment is relevant for a better understanding: #791 (comment) |
|
Looks good to me!
I think you are right here, I think we only use |
That's a fair assessment, imho |
|
I think we're gonna need a custom validator to issue a warning in cases when we have refinements triggered on members that have default values. But I'm happy to defer that, as this PR improves the situation already. |
Already on it 👍 |
In this PR, I attempt to solve #791 which renders product member as if they were required. The issue happens because the Smithy definitions happen in a v1 IDL language version and the shape in question gets
Defaulttrait added.The boolean that indicates whether or not the member should be wrapped in option check for
RequiredorDefaulttrait on the shape, and if they are there, it will make it required.So a non-required field, gets an automatic
Defaultbecause it is defined in a v1 spec, and thus the rendering code scraps the optionality off of it.In this PR, I replace the
Defaulttrait presence check with a more involved one. The check is now:if hasDefault && typeIsNotExternal.I assume a Type.external necessarily mean a refined type (but I'm not 100% sure about that).
In any case, this change behaves
how I want and we can see that in the few examples that are updated in this PR. I added and extra field with
@requiredto make sure it was still respected.