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

fix(core): fix use of references in toJsonString() #3568

Merged
merged 4 commits into from
Aug 8, 2019

Conversation

rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Aug 7, 2019

When tokens are identified to be References, leave their type unchanged
in toJsonString(), so that they'll be properly identified as
references and handled in a potentially cross-stack way.

Add checking to references so that they'll be used correctly in tests.

Fixes #3474


Please read the contribution guidelines and follow the pull-request checklist.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

When tokens are identified to be References, leave their type unchanged
in `toJsonString()`, so that they'll be properly identified as
references and handled in a potentially cross-stack way.

Add checking to references so that they'll be used correctly in tests.
@rix0rrr rix0rrr requested a review from eladb August 7, 2019 09:18
@rix0rrr rix0rrr self-assigned this Aug 7, 2019
const consumingStack = Stack.of(context.scope);
const token = this.replacementTokens.get(consumingStack);
if (!token && this.isCrossStackReference(consumingStack) && !context.preparing) {
throw new Error('Should have hadd a cross-stack reference, but not found. prepare() the reference first.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cryptic error and has a typo

eladb
eladb previously approved these changes Aug 7, 2019
eladb
eladb previously approved these changes Aug 7, 2019
@rix0rrr rix0rrr requested a review from skinny85 as a code owner August 7, 2019 14:30
@rix0rrr rix0rrr added the effort/small Small work item – less than a day of effort label Aug 8, 2019
@mergify mergify bot merged commit 0fc2c3b into master Aug 8, 2019
@RomainMuller RomainMuller deleted the huijbers/jsonstring-references branch August 10, 2019 00:38
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. effort/small Small work item – less than a day of effort
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cross stack references are not identified in stack.toJsonString()
3 participants