Skip to content
This repository has been archived by the owner on Aug 7, 2024. It is now read-only.

Set preload to false on most links #7356

Merged
merged 5 commits into from
Jun 4, 2023
Merged

Set preload to false on most links #7356

merged 5 commits into from
Jun 4, 2023

Conversation

dan-mba
Copy link
Member

@dan-mba dan-mba commented Jun 3, 2023

Changes proposed

The additional JavaScript loaded by preload causes performance issues on mobile.
The NextJS configuration of Webpack will not create a separate main chunk for links that are preloaded on desktop only.
This PR will set all links using our Link component to preload={false}.

Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • My change requires changes to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

@github-actions github-actions bot added medium Pull request with changed lines between 10 and 30 waiting-for-reviewers labels Jun 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2023

Reviewpad Report

ℹ️ Messages

  • A maintainer will review your pull request soon!

@github-actions github-actions bot added small Pull request with less than 10 changed lines and removed medium Pull request with changed lines between 10 and 30 labels Jun 3, 2023
@@ -9,6 +9,7 @@ export default function Link({ children, className, rel, ...restProps }) {
? className
: "text-blue-600 underline decoration-dotted dark:text-blue-500 hover:underline hover:decoration-solid"
}
prefetch={false}
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to have a condition so it only happens on mobile?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried that, but webpack still bundles the code to preload the JavaScript files if the preload is set by a conditional. It is around 700k of JS that gets loaded by the main-XXX chunk of the bundle without preload set to false.

Copy link
Member

Choose a reason for hiding this comment

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

ouch, ok thank you for clarifying 👍

Copy link
Member

@eddiejaoude eddiejaoude left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@eddiejaoude eddiejaoude merged commit 44372da into main Jun 4, 2023
@eddiejaoude eddiejaoude deleted the preload branch June 4, 2023 00:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
small Pull request with less than 10 changed lines waiting-for-reviewers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants