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

Find a way to signal to users that they should never call "resolve" on tokens #634

Closed
eladb opened this issue Aug 28, 2018 · 2 comments
Closed
Labels
@aws-cdk/core Related to core CDK functionality

Comments

@eladb
Copy link
Contributor

eladb commented Aug 28, 2018

Currently, Tokens implement a method resolve which is called to resolve the token during synthesis, but users are tempted to invoke this method in various situations (i.e. to "resolve" the value in runtime, to "resolve" the value so they can reason about it, etc).

We should hide this method and/or signal that it shouldn't be called by users.

I propose that:

  1. We rename this method _resolve.
  2. We make it private and use duck-typing to identify the token (public/private is just a typescript annotation, doesn't apply at runtime).
@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 28, 2018

Both methods hide it from jsii, so jsii users will never be able to implement a new subclass of Token. That seems not ideal.

@eladb
Copy link
Contributor Author

eladb commented Aug 28, 2018

That's a good point, but what is the use case for actually extending tokens by non-framework code? I think it is very limited and maybe even something we want to block, because it's extremely error prone. If there will be a clear use case, we can always provide a base class such as CustomToken that exposes a method to override.

moofish32 added a commit to moofish32/aws-cdk that referenced this issue Aug 28, 2018
@fulghum fulghum added the @aws-cdk/core Related to core CDK functionality label Sep 20, 2018
@eladb eladb closed this as completed Aug 27, 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
Projects
None yet
Development

No branches or pull requests

3 participants