Skip to content

Commit

Permalink
chore: Revert "feat(sns): addSubscription returns the created Subscri…
Browse files Browse the repository at this point in the history
…ption (aws#16785)" (aws#17166)

This reverts commit 62f389e.

Changing the return type from void to Subscription is a breaking change for F# customers, where the return type changes from "unit" to "Subscription", potentially breaking `do` statements.

Example:
https://github.com/aws/aws-cdk/blob/62f389ea0522cbaefca5ca17080228031d401ce6/packages/aws-cdk/lib/init-templates/v1/sample-app/fsharp/src/%25name.PascalCased%25/%25name.PascalCased%25Stack.template.fs#L14

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
njlynch authored and TikiTDO committed Feb 21, 2022
1 parent caced9d commit 42cd69e
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 25 deletions.
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-sns/lib/topic-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export interface ITopic extends IResource, notifications.INotificationRuleTarget
/**
* Subscribe some endpoint to this topic
*/
addSubscription(subscription: ITopicSubscription): Subscription;
addSubscription(subscription: ITopicSubscription): void;

/**
* Adds a statement to the IAM resource policy associated with this topic.
Expand Down Expand Up @@ -68,7 +68,7 @@ export abstract class TopicBase extends Resource implements ITopic {
/**
* Subscribe some endpoint to this topic
*/
public addSubscription(subscription: ITopicSubscription): Subscription {
public addSubscription(subscription: ITopicSubscription) {
const subscriptionConfig = subscription.bind(this);

const scope = subscriptionConfig.subscriberScope || this;
Expand All @@ -83,7 +83,7 @@ export abstract class TopicBase extends Resource implements ITopic {
throw new Error(`A subscription with id "${id}" already exists under the scope ${scope.node.path}`);
}

return new Subscription(scope, id, {
new Subscription(scope, id, {
topic: this,
...subscriptionConfig,
});
Expand Down
22 changes: 0 additions & 22 deletions packages/@aws-cdk/aws-sns/test/sns.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,26 +480,4 @@ describe('Topic', () => {


});

test('result of addSubscription() can be used as a dependency', () => {
// GIVEN
const stack = new cdk.Stack();
const topic = new sns.Topic(stack, 'Topic');
const user = new iam.User(stack, 'User');

// WHEN
const subscription = topic.addSubscription({
bind: () => ({
protocol: sns.SubscriptionProtocol.HTTP,
endpoint: 'http://foo/bar',
subscriberId: 'my-subscription',
}),
});
user.node.addDependency(subscription);

// THEN
Template.fromStack(stack).hasResource('AWS::IAM::User', {
DependsOn: ['Topicmysubscription1E605DD7'],
});
});
});

0 comments on commit 42cd69e

Please sign in to comment.