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

Skipping partial html files generated due to enabling ppr in Nextjs #7625

Merged
merged 21 commits into from
Oct 25, 2024

Conversation

chalosalvador
Copy link
Member

Description

When partial pre-rendering (experimental: {ppr: true} is enabled in next.config.js Next.js includes dynamic pages in the prerender-manifest.json and it generates the .html files with partial html and those files are copied over by firebase to the hosting folder causing the pages to render empty or invalid html pages.

This PR add a check to skip partial html files from being copies over to the hosting folder.

Scenarios Tested

  • Enable ppr in next.config.js
  • Deploy to firebase hosting or run emulators in production mode.
  • Partial html files are skipped and the pages are SSR'd

Sample Commands


const sourcePath = join(contentDist, ...sourcePartsOrIndex);

if (await isPartialHTML(`${sourcePath}.html`)) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's test if .html exists first, that should reduce some of the cost

src/frameworks/next/utils.ts Outdated Show resolved Hide resolved
@chalosalvador chalosalvador enabled auto-merge (squash) October 24, 2024 13:49
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@chalosalvador chalosalvador merged commit 2248973 into master Oct 25, 2024
39 of 41 checks passed
@jamesdaniels jamesdaniels deleted the chalosalvador/fix-next-ppr-dynamic-pages branch October 28, 2024 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants