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(next): only set payload theme preference headers within '/admin' namespace #8078

Open
wants to merge 4 commits into
base: beta
Choose a base branch
from

Conversation

lynndylanhurley
Copy link
Contributor

@lynndylanhurley lynndylanhurley commented Sep 5, 2024

When these headers are set globally, they interfere with the lighthouse CI tool. It forces a redirect that adds 600-800ms to the total load time on mobile.

By only setting the headers for the /admin section of the site, which is where they are used, it resolves the problem for other sections of the site where performance is critical.

Description

  • I have read and understand the CONTRIBUTING.md document in this repository.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Steps to test

  1. Pull down the latest version of the payload 3.0 demo
  2. Run the site in prod mode: pnpm build && pnpm start
  3. Run a lighthouse audit using the cli tool: npx lighthouse@latest http://localhost:3000 --view --chrome-flags="--headless" --verbose
  4. Observe this line in the report:
    Screenshot 2024-09-05 at 2 33 11 PM

Steps to confirm the fix

  1. Open the file: node_modules/@payloadcms/next/dist/withPayload.js
  2. Apply the change from this PR by changing source: '/:path*' to source: '/admin/:path*'
  3. Re-run build and run the site: pnpm build && pnpm start
  4. Re-run the lighthouse check: npx lighthouse@latest http://localhost:3000 --view --chrome-flags="--headless" --verbose
  5. Observe that the redirect perfomance penalty has been resolved

When these headers are set globally, they interfere with the lighthouse CI tool. It forces a redirect that adds 600-800ms to the total load time on mobile.

By only setting the headers for the `/admin` section of the site, which is where they are used, it resolves the problem for other sections of the site where performance is critical.
@lynndylanhurley lynndylanhurley changed the title fix(next): only set payload headers within '/admin' namespace fix(next): only set payload theme preference headers within '/admin' namespace Sep 5, 2024
@lynndylanhurley
Copy link
Contributor Author

Checking in on this. Is there anything more that needs to be done for this to be merge-able?

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.

2 participants