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.

Sign up for GitHub

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

Jump to bottom

Revisit tokens APIs #1933

Closed
eladb opened this issue Mar 4, 2019 · 0 comments · Fixed by #2757
Closed

Revisit tokens APIs #1933

eladb opened this issue Mar 4, 2019 · 0 comments · Fixed by #2757
Assignees
@rix0rrr
Labels
@aws-cdk/core Related to core CDK functionality

Comments

@eladb
Copy link
Contributor

eladb commented Mar 4, 2019

Tokens are a queer beast. We should look into their APIs and make sure they are not easy to abuse.

Thoughts:

  1. Token.unresolved instead of cdk.unresolve
  2. Hide resolve so that it doesn't appear in public API
  3. Factory methods for specific use cases instead of new Token. For example Token.lazy(() => )
  4. Tokens are conceptually coupled to synthesis and now also must be anchored by a construct scope ("lazy" means - resolve during synthesis), maybe we can simply move all token facilities to Construct.node
@eladb eladb added the @aws-cdk/core Related to core CDK functionality label Mar 4, 2019
@eladb eladb removed the package/awscl Cross-cutting issues related to the AWS Construct Library label May 1, 2019
@rix0rrr rix0rrr self-assigned this May 29, 2019
@eladb eladb mentioned this issue Jun 2, 2019
Merged
4 tasks
rix0rrr added a commit that referenced this issue Jun 5, 2019
@rix0rrr
fix(core): refactor use of Tokens
1bef5fc 
Tokens are now represented by the interface `IToken`. The class `Token`
still exists, but it is used to hold static routines for token encoding
(`Token.encodeAsString()`), and as a potential base class for `IToken`
implementations so some default functionality is inherited (this
inheritance is an implementation optimization, it is not a
requirement!).

Actual token use is split into 2 categories:

- *Intrinsics*, represented by `Intrinsic` class: these are to represent values
  that will be understood by the deployment language formalism (e.g.
  CloudFormation), and can be used to escape language type checking.
- *Lazy values*, represented by a `Lazy` class with static factories:
  these will be used to represent type-checked but lazily-produced
  values (evaluated at synthesis time).

In order to be JSII-compatible (which does not currently support
lambdas), `Lazy.stringValue()` et. al. take an interface with a single
method instead of a function.

Also changed in this commit: shoring up property names for encoded
tokens in classes like `CfnParameter` and `CfnElement`.

* `.stringValue` => `.valueAsString`, etc so `.value`, `.valueAsString`,
  `.valueAsList` etc. group together in autocomplete.
* `.ref` now returns a Token, `.refAsString` returns the stringified
  version that token.

To match the other standard the latter should actually be `.refAsString`
but `.ref` is used in so many places that this might be uncomfortable?

Revert `Token.isToken()` to check if the passed object is literally an
`IToken` (to match `Array.isArray()` and other related methods), and
`Token.unresolved()` to check if the passed object might also be an
encoded Token.

Fixes #1933.

BREAKING CHANGES:

* `Token` can no longer be instantiated. Instead, instantiate
  `Intrinsic` or use `Lazy.stringValue` and others.
* `Token.isToken()` will only return true if the object is a Token,
  for detecting arbitrary encoded tokens, use `Token.unresolved()`.
* `CfnOutput`: remove `.ref`, since they can't be referenced anyway.
* `CfnParameter`: rename `.stringValue` => `.valueAsString` (and
  similar for lists and numbers).
* `.ref` now returns an `IToken`, use `.refAsString` to get a
  string-encoded Token.
@rix0rrr rix0rrr mentioned this issue Jun 5, 2019
Merged
4 tasks
rix0rrr added a commit that referenced this issue Jun 11, 2019
@rix0rrr
refactor(core): refactor API of Tokens (#2757)
a12fcfa 
Tokens are now represented by the interface `IResolvable`. The class `Token`
still exists, but it is used to hold static routines for token encoding
(`Token.asString()`).

Actual token use is split into 2 categories:

- *Intrinsics*, represented by static methods on `Token`: these are to represent values
  that will be understood by the deployment language formalism (e.g.
  CloudFormation), and can be used to escape language type checking.
- *Lazy values*, represented by a `Lazy` class with static factories:
  these will be used to represent type-checked but lazily-produced
  values (evaluated at synthesis time).

In order to be JSII-compatible (which does not currently support
lambdas), `Lazy.stringValue()` et. al. take an interface with a single
method instead of a function.

Also changed in this commit: shoring up property names for encoded
tokens in classes like `CfnParameter` and `CfnElement`.

* `.stringValue` => `.valueAsString`, etc so `.value`, `.valueAsString`,
  `.valueAsList` etc. group together in autocomplete.
* `.ref` now returns a Token, `.refAsString` returns the stringified
  version that token.

To match the other standard the latter should actually be `.refAsString`
but `.ref` is used in so many places that this might be uncomfortable?

Fixes #1933.

BREAKING CHANGES:

* `Token` can no longer be instantiated. You probably want to use 
  `Lazy.stringValue` and others, or `Token.asString()` and others.
* `Token.isToken()`/`Token.unresolved()` => `Token.isUnresolved()`.
* `CfnOutput`: remove `.ref`, since they can't be referenced anyway.
* `CfnParameter`: rename `.stringValue` => `.valueAsString` (and
  similar for lists and numbers).
* `.ref` now returns an `IToken`, use `.refAsString` to get a
  string-encoded Token.
* `TokenMap` is now no longer exported, some parts of its functionality
  are available through the static `Tokenization` class.
* `IResolvedValuePostProcessor` => `IResolvableWithPostProcess`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rix0rrr rix0rrr

Labels
@aws-cdk/core Related to core CDK functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor(core): refactor API of Tokens aws/aws-cdk
2 participants
@rix0rrr @eladb