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

feat(ssr): render in Cloud Function, not in build #11459

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

feat(ssr): render in Cloud Function, not in build #11459

wants to merge 11 commits into from

Conversation

LeoMcA
Copy link
Member

@LeoMcA LeoMcA commented Jul 10, 2024 •
edited
Loading

Summary

(MP-1349)

Problem

  • We SSR at build time, which means to deploy changes to yari we have to wait for a full content build (~1 hour) to finish

Solution

How did you test this change?

Deployed to https://test.developer.allizom.org/en-US/, checked across locales/page types with JS enabled/disabled

@caugner caugner changed the title SSR in cloud function rather than in build feat(ssr): render SSR in cloud function, not in build Jul 12, 2024
@caugner caugner changed the title feat(ssr): render SSR in cloud function, not in build feat(ssr): render in Cloud Function, not in build Jul 12, 2024
@LeoMcA LeoMcA force-pushed the cloud-function-ssr branch 2 times, most recently from 562210f to 11cb342 Compare July 12, 2024 14:52
@LeoMcA LeoMcA marked this pull request as ready for review July 12, 2024 14:59
@LeoMcA LeoMcA requested review from mdn-bot and a team as code owners July 12, 2024 14:59
caugner
caugner reviewed Jul 12, 2024
View reviewed changes
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

Overall LGTM, but some questions:

cloud-function/package.json Show resolved Hide resolved
cloud-function/src/handlers/render-html.ts Outdated Show resolved Hide resolved
cloud-function/src/handlers/render-html.ts Show resolved Hide resolved
cloud-function/tsconfig.json Show resolved Hide resolved
caugner
caugner approved these changes Jul 22, 2024
View reviewed changes
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

LGTM, just one nit.

cloud-function/src/handlers/render-html.ts Outdated Show resolved Hide resolved
@LeoMcA LeoMcA marked this pull request as draft July 22, 2024 09:54
@github-actions github-actions bot added the merge conflicts 🚧 Please rebase onto or merge the latest main. label Aug 9, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 9, 2024

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot added the idle label Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caugner caugner caugner approved these changes

@mdn-bot mdn-bot Awaiting requested review from mdn-bot mdn-bot is a code owner

Assignees
No one assigned
Labels
cloud-function github-actions idle merge conflicts 🚧 Please rebase onto or merge the latest main.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@LeoMcA @caugner