fix: improving the lighthouse score #3368
Conversation
lakshaydewan
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg and
magicmatatjahu
as code owners
WalkthroughThe changes in this pull request involve modifications to the Next.js application, specifically in how scripts and external resources are managed. The Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3368--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
pages/_app.tsx (1)
48-48: Add newline at end of file.Add a newline character at the end of the file to comply with linting rules.
export default appWithTranslation(MyApp); +🧰 Tools
🪛 eslint
[error] 48-48: Newline required at end of file but not found.
(eol-last)
[error] 48-48: Insert
⏎(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (7)
public/img/social/cli-card.jpgis excluded by!**/*.jpgpublic/img/social/generator-card.jpgis excluded by!**/*.jpgpublic/img/social/githubactions-card.jpgis excluded by!**/*.jpgpublic/img/social/meetings.pngis excluded by!**/*.pngpublic/img/social/modelina-card.jpgis excluded by!**/*.jpgpublic/img/social/parsers-card.jpgis excluded by!**/*.jpgpublic/img/social/website-card.jpgis excluded by!**/*.jpg
📒 Files selected for processing (3)
pages/_app.tsx(3 hunks)pages/_document.tsx(2 hunks)styles/globals.css(0 hunks)
💤 Files with no reviewable changes (1)
- styles/globals.css
🧰 Additional context used
🪛 eslint
pages/_app.tsx
[error] 48-48: Newline required at end of file but not found.
(eol-last)
[error] 48-48: Insert ⏎
(prettier/prettier)
pages/_document.tsx
[error] 23-23: Replace "preconnet" with 'preconnet'
(prettier/prettier)
[error] 23-23: Unexpected usage of doublequote.
(jsx-quotes)
[error] 24-24: Replace "preconnect" with 'preconnect'
(prettier/prettier)
[error] 24-24: Unexpected usage of doublequote.
(jsx-quotes)
[error] 26-26: Insert ·
(prettier/prettier)
[error] 28-28: Replace "preload" with 'preload'
(prettier/prettier)
[error] 28-28: Unexpected usage of doublequote.
(jsx-quotes)
🔇 Additional comments (4)
pages/_app.tsx (1)
5-5: LGTM: Proper import of Next.js Script component.
The import is correctly placed and follows Next.js best practices for script management.
pages/_document.tsx (3)
Line range hint 3-12: LGTM: Clean implementation of Document setup
The addition of the Script import and getInitialProps implementation follows Next.js best practices.
🧰 Tools
🪛 eslint
[error] 1-5: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 7-8: Delete ⏎
(prettier/prettier)
[error] 7-8: More than 1 blank line not allowed.
(no-multiple-empty-lines)
26-30: Great performance optimization with font preloading
The switch to preloading fonts is an excellent optimization that aligns with the PR's objective to improve the Lighthouse score. This change will help reduce the First Contentful Paint (FCP) by loading critical fonts earlier in the page lifecycle.
🧰 Tools
🪛 eslint
[error] 26-26: Insert ·
(prettier/prettier)
[error] 28-28: Replace "preload" with 'preload'
(prettier/prettier)
[error] 28-28: Unexpected usage of doublequote.
(jsx-quotes)
37-37: Excellent optimization using Next.js Script component
The replacement of the regular script tag with Next.js's Script component and lazyOnload strategy is a great optimization. This ensures that the GitHub buttons script doesn't block the initial page load, contributing to the improved Lighthouse score.
| {/* <MDXProvider components={mdxComponents}> */} | ||
| <Head> | ||
| <script async defer src='https://buttons.github.io/buttons.js'></script> | ||
| <Script async defer src='https://buttons.github.io/buttons.js' strategy='lazyOnload'></Script> |
There was a problem hiding this comment.
💡 Codebase verification
Duplicate GitHub button script detected - needs consolidation
The GitHub button script is currently loaded twice:
- In
pages/_app.tsx - In
pages/_document.tsx
Both instances use the Next.js Script component with lazyOnload strategy, but loading the same script twice is unnecessary and could impact performance. You should:
- Keep the script in
pages/_document.tsxsince it's the more appropriate location for global scripts - Remove the duplicate script from
pages/_app.tsx
🔗 Analysis chain
Excellent performance optimization using Next.js Script component!
The implementation correctly uses the lazyOnload strategy which is ideal for non-critical third-party scripts like GitHub buttons. This change will help improve the First Contentful Paint (FCP) by deferring script loading.
Let's verify if there are other GitHub button scripts that should be updated for consistency:
Consider moving third-party scripts to a dedicated component (e.g., components/ThirdPartyScripts.tsx) to better organize external dependencies. This would make it easier to manage and audit third-party scripts in the future.
Let me check the content of pages/_document.tsx to see if there's a duplicate GitHub button script that needs to be addressed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other potential GitHub button script tags that might need similar optimization
rg -l 'buttons\.github\.io/buttons\.js'
# Search for any remaining basic script tags that could be optimized
ast-grep --pattern 'script[src]'
Length of output: 111
Script:
#!/bin/bash
# Check content of _document.tsx for GitHub button script implementation
rg -A 2 -B 2 'buttons\.github\.io/buttons\.js' pages/_document.tsx
Length of output: 382
| <link rel="preconnet" href='https://fonts.googleapis.com' /> | ||
| <link rel="preconnect" href='https://fonts.gstatic.com' crossOrigin='anonymous' /> |
There was a problem hiding this comment.
Fix typo and formatting issues
There are a few minor issues to address:
- Line 23: "preconnet" should be "preconnect"
- Line 26: Missing space before href attribute
Apply this diff to fix the issues:
- <link rel="preconnet" href='https://fonts.googleapis.com' />
+ <link rel="preconnect" href='https://fonts.googleapis.com' />
- <link rel='preload'href='https://cdn.jsdelivr.net/gh/tonsky/FiraCode@4/distr/fira_code.css' />
+ <link rel='preload' href='https://cdn.jsdelivr.net/gh/tonsky/FiraCode@4/distr/fira_code.css' />Also applies to: 26-26
🧰 Tools
🪛 eslint
[error] 23-23: Replace "preconnet" with 'preconnet'
(prettier/prettier)
[error] 23-23: Unexpected usage of doublequote.
(jsx-quotes)
[error] 24-24: Replace "preconnect" with 'preconnect'
(prettier/prettier)
[error] 24-24: Unexpected usage of doublequote.
(jsx-quotes)
2 participants
Improved the lighthouse score on mobile devices by preloading fonts and using Native Script tag from next/script
the FCP of the website went down from 3 seconds average to 1.5 seconds improving the performance greatly.
Summary by CodeRabbit
New Features
<Script>component andlazyOnloadstrategy.Bug Fixes
Chores