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(nextjs-component, nextjs-cdk-construct): create AWS resources for dynamic SSG (#1476) #1477

Merged
merged 1 commit into from
Aug 3, 2021

Conversation

laiso
Copy link
Contributor

@laiso laiso commented Aug 1, 2021

it adds the resource creation conditions that my suggestions for fix #1476. I think we need to create SQS and Regeneration Handler at deployment time, because it can 'fallback: true|blocking' with 'revaliate'.

I would like your review on whether that is an appropriate solution or not. thanks

@slsnextbot
Copy link
Collaborator

slsnextbot commented Aug 1, 2021

Handler Size Report

No changes to handler sizes.

Base Handler Sizes (kB) (commit bdef74d)

{
    "Lambda@Edge": {
        "Default Lambda": {
            "Standard": 1227,
            "Minified": 433
        },
        "API Lambda": {
            "Standard": 86,
            "Minified": 34
        },
        "Image Lambda": {
            "Standard": 894,
            "Minified": 354
        },
        "Regeneration Lambda": {
            "Standard": 620,
            "Minified": 220
        }
    }
}

New Handler Sizes (kB) (commit a28ec93)

{
    "Lambda@Edge": {
        "Default Lambda": {
            "Standard": 1227,
            "Minified": 433
        },
        "API Lambda": {
            "Standard": 86,
            "Minified": 34
        },
        "Image Lambda": {
            "Standard": 894,
            "Minified": 354
        },
        "Regeneration Lambda": {
            "Standard": 620,
            "Minified": 220
        }
    }
}

@codecov
Copy link

codecov bot commented Aug 1, 2021

Codecov Report

Merging #1477 (a4b8ecd) into master (a5d74d2) will decrease coverage by 0.01%.
The diff coverage is 96.87%.

❗ Current head a4b8ecd differs from pull request most recent head a28ec93. Consider uploading reports for the commit a28ec93 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1477      +/-   ##
==========================================
- Coverage   84.15%   84.14%   -0.02%     
==========================================
  Files          96       97       +1     
  Lines        3371     3381      +10     
  Branches     1006     1009       +3     
==========================================
+ Hits         2837     2845       +8     
- Misses        474      476       +2     
  Partials       60       60              
Impacted Files Coverage Δ
...ackages/libs/s3-static-assets/src/lib/constants.ts 100.00% <ø> (ø)
...rless-components/nextjs-component/src/component.ts 89.96% <75.00%> (-0.32%) ⬇️
packages/libs/lambda-at-edge/src/build.ts 96.66% <94.44%> (-0.43%) ⬇️
packages/libs/core/src/route/index.ts 100.00% <100.00%> (ø)
packages/libs/core/src/route/locale.ts 99.00% <100.00%> (ø)
...-at-edge/src/build/third-party/integration-base.ts 100.00% <100.00%> (ø)
...mbda-at-edge/src/build/third-party/next-i18next.ts 100.00% <100.00%> (ø)
...ackages/libs/lambda-at-edge/src/default-handler.ts 89.28% <100.00%> (+0.19%) ⬆️
...ibs/lambda-at-edge/src/lib/filterOutDirectories.ts 100.00% <100.00%> (ø)
.../libs/lambda-at-edge/src/lib/readDirectoryFiles.ts 100.00% <100.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5d74d2...a28ec93. Read the comment docs.

@dphang
Copy link
Collaborator

dphang commented Aug 2, 2021

Looks fine to me, though please fix the style issue as reported by codacy.

Also can you check if it is needed in the CDK deployer as well? (can be in another PR if needed)

@laiso laiso changed the title fix(nextjs-component): create AWS resources for dynamic SSG (#1476) fix(nextjs-component, nextjs-cdk-construct): create AWS resources for dynamic SSG (#1476) Aug 2, 2021
@laiso
Copy link
Contributor Author

laiso commented Aug 2, 2021

@dphang thanks you. I updated the commit.

I also fixed the nextjs-cdk-construct. actually it reads to different manifest, the conditions also change. As below

jq . < .serverless_nextjs/default-lambda/prerender-manifest.json

  "routes": {
    "/": {
      "initialRevalidateSeconds": false,
      "srcRoute": null,
      "dataRoute": "/_next/data/O26akBLbm_uS3eQK6COu5/index.json"
    }
  },
  "dynamicRoutes": {
    "/posts/[slug]": {
      "routeRegex": "^/posts/([^/]+?)(?:/)?$",
      "dataRoute": "/_next/data/O26akBLbm_uS3eQK6COu5/posts/[slug].json",
      "fallback": "/posts/[slug].html",
      "dataRouteRegex": "^/_next/data/O26akBLbm_uS3eQK6COu5/posts/([^/]+?)\\.json$"
    }
  },

@dphang
Copy link
Collaborator

dphang commented Aug 2, 2021

Thanks, though it looks like the build is still failing from cdk-construct due to typing issue..

@dphang dphang merged commit 7e9c923 into serverless-nextjs:master Aug 3, 2021
@laiso
Copy link
Contributor Author

laiso commented Aug 3, 2021

Now I saw it merged. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

requires set at least one SSG path in working ISR on AWS
3 participants