feat(cli): cannot pass objects and numbers as context arguments #20068
feat(cli): cannot pass objects and numbers as context arguments #20068mergify[bot] merged 4 commits intoaws:mainfrom corymhall:cli-future-flag-context
Conversation
All future flag context arguments are either `true` or `false` (enabled or disabled). When passing future flags as command line arguments, i.e. ``` cdk deploy --context @aws-cdk/core:newStyleStackSynthesis=false cdk deploy --context @aws-cdk/core:newStyleStackSynthesis=0 ``` The values are interpreted as strings so the above two values get interpreted as 'false' and '0' which are truthy (and not what we want). This PR adds a function to parse context arguments and covert any future flags to boolean values. 'false' and '0' will be evaluated as `false`. This functionality could be beneficial to more than just future flags, but with future flags we know that the value must be a boolean.
|
|
rix0rrr
left a comment
There was a problem hiding this comment.
Instead of special-casing feature flags, I was hoping to generalize the context parsing for all values in the CLI. Is that possible?
Ok i've updated it to try and parse all values. Let me know what you think. |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…20068) All context arguments passed to the cli are parsed as strings. ``` cdk deploy --context @aws-cdk/core:newStyleStackSynthesis=false cdk deploy --context @aws-cdk/core:newStyleStackSynthesis=0 ``` The values are interpreted as strings so the above two values get interpreted as 'false' and '0' which are truthy (and not what we want). This PR adds a function to parse context arguments and try to covert values into their correct type. ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
Interestingly this fix broke things for me where I was expecting a string and evaluating to a boolean. With this change --context IsSomething=true evaluated to false where it previously evaluated to true as I was doing this in my code I was able to resolve it on my end by doing one of the below (treat it as boolean or as a literal string)
|
|
Isn't everything after the > typeof JSON.parse('1')
'number'
> typeof JSON.parse('1.2')
'number'
> typeof JSON.parse('true')
'boolean'
> typeof JSON.parse('false')
'boolean' |
|
I type cast in my code as I updated my code to accommodate this change, but was surprised when it occurred as suddenly all of my stacks changed when pipelines in my org ran post 7/28. |
All context arguments passed to the cli are parsed as strings.
The values are interpreted as strings so the above two values get
interpreted as 'false' and '0' which are truthy (and not what we want).
This PR adds a function to parse context arguments and try to covert values
into their correct type.
All Submissions:
Adding new Unconventional Dependencies:
New Features
yarn integto deploy the infrastructure and generate the snapshot (i.e.yarn integwithout--dry-run)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license