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

Improve Page Load Performance by Deferring Scripts and Lazy Loading Images ✈️ #1177

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sanjaiyan-dev
Copy link

Hi, I believe this pull request implements small but valuable performance improvements and follows best web practices.
The script tag is now run with the defer attribute, which improves performance. For more information, please see https://web.dev/articles/efficiently-load-third-party-javascript#defer.
We've added the loading attribute to images below the viewport, enabling lazy loading and optimizing network usage. For more information, please refer to https://web.dev/articles/browser-level-image-lazy-loading#the_loading_attribute.

Copy link
Contributor

@senekor senekor left a comment

Choose a reason for hiding this comment

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

@sanjaiyan-dev It's been a while, but I tested the changes and will merge if you'd like to rebase this PR.

@apiraino
Copy link
Contributor

Do we have some numbers about the perf. diff (+/-) after this patch?

@senekor
Copy link
Contributor

senekor commented Jan 23, 2025

No and I don't think there would be any if we actually measured. The images are just a few logos with a few kilobytes each. Internet connection would have to be really bad for that to be significant. I did verify that the images are in fact only loaded when you scroll to the bottom of the page.

I was leaning towards merging unless there is a reason not to, because the change is good practice on the face of it. Do you think we shouldn't merge unless there is a concrete benefit?

@apiraino
Copy link
Contributor

I was leaning towards merging unless there is a reason not to, because the change is good practice on the face of it. Do you think we shouldn't merge unless there is a concrete benefit?

I'm mostly neutral about this patch. I was just interested in supporting the claimed perf. improvement. IIRC the browser developer tools allow restraining bandwidth also for this kind of testing (don't have time to investigate rn).

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.

3 participants