-
-
Notifications
You must be signed in to change notification settings - Fork 15
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
Implement lazy loading for third-party JavaScript to improve page load speeds #511
Conversation
…rendering with a 2 seconds delay
…rendering with a 3 seconds delay and animation request
WalkthroughThe changes introduce a Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
app/root.tsx (2)
247-282
: Approve with suggestions: Lazy loading implementation for third-party scriptsThe implementation of lazy loading for Google Tag Manager and Ezoic scripts is a good approach to improve initial page load times. However, there are some areas for potential improvement:
- Consider using a more dynamic approach for the delay, such as after the main content has loaded or based on network conditions.
- Implement error handling for script loading failures.
- Consider the impact on analytics data accuracy for very short user sessions.
To improve readability and maintainability, consider refactoring the useEffect hook into smaller, more focused functions:
const loadGoogleTagManager = () => { // GTM loading logic }; const loadEzoicScript = () => { // Ezoic loading logic }; useEffect(() => { const timer = setTimeout(() => { window.requestAnimationFrame(() => { loadGoogleTagManager(); loadEzoicScript(); }); }, 3000); return () => clearTimeout(timer); }, []);This refactoring will make the code easier to understand and maintain.
247-282
: Summary: Lazy loading implementation aligns with PR objectivesThe implementation of lazy loading for Google Tag Manager and Ezoic scripts aligns well with the PR objectives of improving page load speeds and addressing mobile performance issues. By delaying the loading of these third-party scripts, the initial page load time should improve, potentially leading to better Core Web Vitals scores and an enhanced mobile experience.
However, to fully realize the benefits and prepare for Google's mobile-first indexing initiative, consider the following recommendations:
- Implement more dynamic script loading based on user interaction or network conditions.
- Add error handling and fallback mechanisms for script loading failures.
- Use Subresource Integrity (SRI) for enhanced security when loading third-party scripts.
- Consider using
requestIdleCallback
for more intelligent script loading timing.- Monitor and analyze the impact on Core Web Vitals scores, especially Largest Contentful Paint (LCP) and First Input Delay (FID), to ensure the changes are having the desired effect.
These enhancements will further improve the user experience and help ensure readiness for Google's mobile-first indexing after July 5, 2024.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- app/root.tsx (1 hunks)
🔇 Additional comments not posted (1)
app/root.tsx (1)
263-266
: Security consideration: Third-party script loadingLoading scripts from third-party sources, like Ezoic, introduces potential security risks if the source is compromised. Consider implementing Subresource Integrity (SRI) to ensure the integrity of the loaded script:
const script = document.createElement('script'); script.src = '//www.ezojs.com/ezoic/sa.min.js'; script.integrity = 'sha384-HASH_VALUE_HERE'; // Replace with actual hash script.crossOrigin = 'anonymous'; script.async = true; document.body.appendChild(script);To verify the current integrity of the Ezoic script, you can run the following command:
This will generate the integrity hash that you can use in the script tag.
app/root.tsx
Outdated
useEffect(() => { | ||
const timer = setTimeout(() => { | ||
window.requestAnimationFrame(() => { | ||
// Load Google Tag Manager | ||
;(function (w, d, s, l, i) { | ||
w[l] = w[l] || [] | ||
w[l].push({ 'gtm.start': new Date().getTime(), event: 'gtm.js' }) | ||
var f = d.getElementsByTagName(s)[0], | ||
j = d.createElement(s), | ||
dl = l != 'dataLayer' ? '&l=' + l : '' | ||
j.async = true | ||
j.src = 'https://www.googletagmanager.com/gtm.js?id=' + i + dl | ||
f.parentNode.insertBefore(j, f) | ||
})(window, document, 'script', 'dataLayer', 'GTM-WH4KFG5') | ||
|
||
// Load Ezoic script | ||
const script = document.createElement('script') | ||
script.src = '//www.ezojs.com/ezoic/sa.min.js' | ||
script.async = true | ||
document.body.appendChild(script) | ||
|
||
script.onload = function () { | ||
window.ezstandalone = window.ezstandalone || {} | ||
ezstandalone.cmd = ezstandalone.cmd || [] | ||
ezstandalone.cmd.push(function () { | ||
ezstandalone.define(118, 116) | ||
ezstandalone.refresh() | ||
ezstandalone.enable() | ||
ezstandalone.display() | ||
}) | ||
} | ||
}) | ||
}, 3000) // Delay of 3000ms (3 second) | ||
|
||
return () => clearTimeout(timer) | ||
}, []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Performance suggestion: Consider using requestIdleCallback
To further optimize performance and ensure analytics accuracy, consider using requestIdleCallback
instead of a fixed timeout. This will load the scripts when the browser is idle, potentially earlier than 3 seconds if possible, or later if the main thread is busy:
useEffect(() => {
const loadScripts = () => {
window.requestAnimationFrame(() => {
loadGoogleTagManager();
loadEzoicScript();
});
};
if ('requestIdleCallback' in window) {
window.requestIdleCallback(loadScripts, { timeout: 3000 });
} else {
// Fallback for browsers that don't support requestIdleCallback
setTimeout(loadScripts, 3000);
}
}, []);
This approach balances performance optimization with ensuring the scripts load within a reasonable timeframe.
Deploying saddlebag-with-pockets with Cloudflare Pages
|
Quality Gate passedIssues Measures |
Changes
This PR introduces lazy loading for third-party JavaScript, specifically advertisements and Google Analytics, to improve page load speeds. The changes include:
App
component withinapp/root.tsx
setTimeout
andrequestAnimationFrame
to manage the delayed loading of Google Tag Manager and Ezoic scriptsKey updates:
requestAnimationFrame
to further optimize the loading processMotivation
These changes aim to improve the initial load time and overall performance of our application, especially on mobile devices. This is crucial because:
This should result in:
Related Issues
Addresses #507 - Improve page load speeds
Summary by CodeRabbit
New Features
Bug Fixes
Style