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

Consider making Construct stack-aware #798

Closed
eladb opened this issue Sep 27, 2018 · 4 comments · Fixed by #1753
Closed

Consider making Construct stack-aware #798

eladb opened this issue Sep 27, 2018 · 4 comments · Fixed by #1753
Assignees
Labels
@aws-cdk/core Related to core CDK functionality effort/small Small work item – less than a day of effort feature-request A feature should be added or improved.

Comments

@eladb
Copy link
Contributor

eladb commented Sep 27, 2018

We are starting to realize that the fact the Construct is not stack-aware hinders our ability to provide a good experience. For example, to get a handle to the stack, users need to write Stack.find(this) instead of something like this.stack. Similarly, if I want to find a resource, I can use this.findChild but have to cast it down to cdk.Resource. Would have been handy to have this.findResource, etc.

A possible simple solution to this is to convert the base Construct to BaseConstruct and then define a new class Construct which will be Stack and Resource aware and will have the desired APIs.

Brought up by @ccurrie-amzn in a discussion over #784

@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 28, 2018

I would have loved to have something like this before, but I was under the impression the rationale for not having it was to avoid tying ourselves to CloudFormation concepts.

@eladb
Copy link
Contributor Author

eladb commented Oct 3, 2018

That’s correct, the question is whether we define a Stack as a CloudFormation concept or a more general one.

@eladb eladb added enhancement @aws-cdk/core Related to core CDK functionality labels Dec 17, 2018
@srchase srchase added feature-request A feature should be added or improved. and removed enhancement labels Jan 3, 2019
@rix0rrr
Copy link
Contributor

rix0rrr commented Jan 8, 2019

As an implementation note, StackElement already has a stack property.

@RomainMuller RomainMuller self-assigned this Feb 13, 2019
@RomainMuller
Copy link
Contributor

I think making stack visible on the .nodes property does not create a strong decision on whether Stack is a CloudFormation or CDK concept. Indeed, it pertains to the top-level Stack class, which we can still have mean anything we'd like, and it could synthesize into a CloudFormation stack AND some not-CloudFormation things.

Anyhow - I'm adding those couple of convenience accessors, because I'm real tired of Stack.find(this).formatArn({ ...});

RomainMuller added a commit that referenced this issue Feb 13, 2019
Makes it easier to access some construct's `Stack`, avoiding
having to sprinkle `Stack.find(this)` everywhere. While there,
added `construct.node.formatArn` and `construct.node.parseArn`
as convenience methods to avoid indirecting via the `Stack`.

BREAKING CHANGE: `Stack.find(c)` and `Stack.tryFind(c)` were
replaced by `c.node.stack`.

Fixes #798
RomainMuller added a commit that referenced this issue Feb 13, 2019
Makes it easier to access some construct's `Stack`, avoiding
having to sprinkle `Stack.find(this)` everywhere.

BREAKING CHANGE: `Stack.find(c)` and `Stack.tryFind(c)` were
replaced by `c.node.stack`.

Fixes #798
@fulghum fulghum added the effort/small Small work item – less than a day of effort label Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality effort/small Small work item – less than a day of effort feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants