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

(aws-amplify-alpha): (Missing cloudformation property framework for Branch) #23325

Closed
philschmid opened this issue Dec 13, 2022 · 7 comments
Closed
Assignees
Labels
@aws-cdk/aws-amplify Related to AWS Amplify effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@philschmid
Copy link

Describe the bug

The Branch construct is missing the important Framework property from cloudformation, which is needed to run NextJs successfully. Since it is not automatically detected

Expected Behavior

Allowing to add framework property

Current Behavior

Not possible

Reproduction Steps

Create Branch try to add framework property to BranchOptions

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.51.1 (build 3d30cdb)

Framework Version

^2.51.1-alpha.0

Node.js Version

v16.13.0

OS

macos

Language

Typescript

Language Version

3.9.7

Other information

No response

@philschmid philschmid added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 13, 2022
@github-actions github-actions bot added the @aws-cdk/aws-amplify Related to AWS Amplify label Dec 13, 2022
@peterwoodworth
Copy link
Contributor

We offer this on our L1 CfnBranch, just not the L2 which is still considered in development

I'm going to remark this as a feature request for the L2 to support the framework property.

In the meantime you can still use the L2 and set the L1 property with an escape hatch

const branch = new Branch(...);
(branch.node.defaultChild as CfnBranch).addPropertyOverride('Framework', ...);

@peterwoodworth peterwoodworth added good first issue Related to contributions. See CONTRIBUTING.md p2 feature-request A feature should be added or improved. effort/small Small work item – less than a day of effort and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 13, 2022
@philschmid
Copy link
Author

Thank you for the response @peterwoodworth. I am using the suggested workaround already.

@kaizencc
Copy link
Contributor

We'll pick this up. @sumupitchayan will take a stab at this one.

@calavera
Copy link

calavera commented Jan 30, 2023

You'll also need the platform property in the amplify.App construct to accomplish what this issue describes:

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-amplify-app.html#:~:text=%3A%20No%20interruption-,Platform,-The%20platform%20for

This would be a quick example of configuring an Amplify app to support Next.js 12 and above:

const app = new amplify.App(scope, 'App', { platform: 'WEB_COMPUTE' });
const branch = app.addBranch('main', { framework: 'Next.js' }); 

@sumupitchayan
Copy link
Contributor

sumupitchayan commented Jan 30, 2023

You'll also need the platform property in the amplify.App construct to accomplish what this issue describes:

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-amplify-app.html#:~:text=%3A%20No%20interruption-,Platform,-The%20platform%20for

This would be a quick example of configuring an Amplify app to support Next.js 12 and above:

const app = new amplify.App(scope, 'App', { platform: 'WEB_COMPUTE' });
const branch = app.addBranch('main', { framework: 'Next.js' }); 

After our conversation with Amplify, we learned that the framework property is meant to be an open string that we can take in any value (if an invalid framework is entered, Amplify just ignores it and does not break).

I will go ahead an update my PR to include the platform property in the amplify.App construct to solve this issue.

mergify bot pushed a commit that referenced this issue Aug 25, 2023
### Problem:
The Amplify App L2 construct does not support the ability for customers to configure a SSR app without reaching down into the lower level L1.

### Solution:
Expose the `platform` field used to set SSG vs SSR hosted applications. Do not expose `WEB_DYNAMIC` as SSRv1 is on the deprecation path.

### Testing Done:
* `yarn build+test`
* added new tests for `platform` field
* updated snapshots for new default value

Closes #24076 and #23325

----

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

github-actions bot commented Mar 8, 2024

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@malikalimoekhamedov
Copy link

addBranch

Is it 'Next.js' or 'Next.js - SSR'?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-amplify Related to AWS Amplify effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants