-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat: cloudformation condition chaining #1494
Conversation
BREAKING CHANGE: the deprecated `cloudformation.XxxResource` classes have been removed. Use the `CfnXxx` classes instead.
Change `conditionXx` methods to accept an interface `IConditionExpression` instead of concrete class and implement this interface by the `Condition` construct. This enables chaining conditions like so: const cond1, cond2, cond, cond4 = new cdk.Condition(...) Fn.conditionOr(cond1, cond2, Fn.conditionAnd(cond3, cond4)) Fixes #1457
/** | ||
* Returns a JSON node that represents this condition expression | ||
*/ | ||
resolve(): any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why resolve()
and not a more specific fucntion name? If you wanted resolve()
you could have accepted a Token, or a ConditionBase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I wanted the Condition construct to implement it, so I had to define it as an interface. And I want the recursive resolution behavior of tokens to apply to full expressions, so I just faked it.
We should probably define an IToken
with a resolve method so (reopened #79)
super({ [type]: value }); | ||
} | ||
|
||
public toConditionJson() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this called?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops. will remove
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
Change
conditionXx
methods to accept an interfaceIConditionExpression
instead of concrete class and implement this interface by the
Condition
construct.
This enables chaining conditions like so:
Fixes #1457
Pull Request Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.