fix: Replace try() with can(coalesce()) to better handle null values
#392
Conversation
…alues The default behaviour of `try` will return a `null` value if present. So update the logic to use `can(coalesce())` which will ignore a `null` value.
| @@ -92,7 +92,7 @@ resource "aws_lb_listener" "this" { | |||
| certificate_arn = try(each.value.certificate_arn, null) | |||
|
|
|||
| dynamic "default_action" { | |||
| for_each = try([each.value.authenticate_cognito], []) | |||
| for_each = can(coalesce(each.value.authenticate_cognito)) ? [each.value.authenticate_cognito] : [] | |||
There was a problem hiding this comment.
this is not a change that we are going to accept. you should change your variables in your wrapper to make it work with the existing implementation. I would start with not using null and instead default to an empty list.
There was a problem hiding this comment.
I'm confused though... The change is a safer null-safe implementation with the same end result...
From memory I tried a few different implementations of the object and couldn't find a way of not having the null value come through...
Will setup a reproducer to confirm....
There was a problem hiding this comment.
The change is a safer null-safe implementation
I don't know what is meant by this. but in general, you are applying defaults that conflict with the module - therefore, you should adapt your implementation, not the other way around
Description
The default behaviour of
trywill return anullvalue if present. So update the logic to usecan(coalesce())which will ignore anullvalue.Motivation and Context
Fixes #390
Breaking Changes
None
How Has This Been Tested?
examples/*to demonstrate and validate my change(s)examples/*projectspre-commit run -aon my pull request