Skip to content
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

Intrinsic functions and pseudo parameters hard to find, confusing to use #202

Closed
rclark opened this issue Jun 30, 2018 · 4 comments · Fixed by #1428
Closed

Intrinsic functions and pseudo parameters hard to find, confusing to use #202

rclark opened this issue Jun 30, 2018 · 4 comments · Fixed by #1428
Labels
feature-request A feature should be added or improved. pr/breaking-change This PR is a breaking change. It needs to be modified to be allowed in the current major version.

Comments

@rclark
Copy link

rclark commented Jun 30, 2018

It might be worth considering splitting up '@aws-cdk/core' into smaller libraries. The reference docs are hard to browse because so much is included in this library, and I tend to end up with a very long list of imports from this lib.

For example, it might be worth breaking out intrinsic functions and pseudo parameters. Quick and easy access to these tends to be pretty important in my experience with Cloudformation.

Related, I find that defining both intrinsic functions and pseudo parameters as classes is a pretty bad fit to my mental model of what these represent. I would prefer to interact with pseudo parameters as though they were variables, and intrinsic functions like... functions.

For example,

new logs.LogGroupResource(stack, 'Logs', {
  logGroupName: new FnSub('/aws/lambda/${AWS::StackName}') as any,
  retentionInDays: 14
});

Not only do I have to use as any (maybe that's not as confusing to folks used to TypeScript), but new FnSub() seems odd. You might consider implementing function wrappers for these objects? I would find that more intuitive.

const fnSub = (name) => new FnSub(name);

For pseudo parameters:

new Lambda(stack, "Function", {
  functionName: new AwsStackName() as any,
  code: new LambdaS3Code(utilityBucket, `bundles/aws-cdk-explorer/${sha}.zip`),
  handler: 'index.js',
  runtime: LambdaRuntime.NodeJS
});

new AwsStackName() as any is just a lot to type for something that I think of as simply a variable. It'd be more meaningful to me if I could just reference awsStackName and that would give me the {"Ref": "AWS::StackName"} construct in the output template. If there was something like an "pseudo parameters functions" class:

class PseudoParameters {
  get awsStackName() { return new AwsStackName(); }
}
@eladb
Copy link
Contributor

eladb commented Jun 30, 2018

Totally agree about modeling intrinsic functions and pseudo parameters more idiomatically (also also about extracting those to a separate @aws-cdk/cloudformation library.

(We had some limitations in our cross language support tech that limited us to using classes, but these limitations have since been lifted and I believe that modeling intrinsics and pseudo params as static functions/variables makes sense)

I believe we can do this:

import { Fn, Pseudo } from '@aws-cdk/cloudformation';

Fn.join(delimiter, arr)
Fn.findInMap(mapping, key, attr)
Pseudo.region
Pseudo.accountId

Would that be more intuitive?

@rclark
Copy link
Author

rclark commented Jun 30, 2018

Yes, that would definitely feel more intuitive.

@eladb eladb added enhancement pr/breaking-change This PR is a breaking change. It needs to be modified to be allowed in the current major version. labels Dec 17, 2018
eladb pushed a commit that referenced this issue Dec 27, 2018
Refactor CloudFormation intrinsic functions so that they will have a more idiomatic shape. They have been converted from classes (e.g. `new FnJoin(...)`) to static methods (e.g. `Fn.join(...)`). Condition functions are mapped to `Fn.conditionXxx` and return an `FnCondition` object.

Furthermore, return type of these static methods are now tokens represented as the resolved type (i.e. `string` or `string[]`) instead of `CloudFormationToken` objects. This allows using these functions and pseudo parameters naturally (instead of requiring the use of `.toString()` or `.toList()`).

Fixes #202

BREAKING CHANGE: CloudFormation intrinsic functions are now represented as static methods under the `Fn` class (e.g. `Fn.join(...)` instead of `new FnJoin(...).toString()`).
@srchase srchase added feature-request A feature should be added or improved. and removed enhancement labels Jan 3, 2019
@mirskiy
Copy link
Contributor

mirskiy commented Apr 25, 2019

In case anyone's still having trouble finding pseudo parameters and stumbles upon this issue, they are currently (0.29.0) in cdk.Aws and work nicely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. pr/breaking-change This PR is a breaking change. It needs to be modified to be allowed in the current major version.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants