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

Load the main JS bundle in production with async #1485

Merged
merged 4 commits into from
Mar 24, 2017

Conversation

arunoda
Copy link
Contributor

@arunoda arunoda commented Mar 23, 2017

Fixes #1436

With this, we don't need to wait for the JS bundle and page can render directly.
This will lead to better page load performance.

arunoda referenced this pull request Mar 23, 2017
Load script with the defer attribute
@ptomasroos
Copy link
Contributor

Great improvments @arunoda

Copy link
Member

@timneutkens timneutkens left a comment

Choose a reason for hiding this comment

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

Niiiiice 😍 ❤️


async function writeBuildStats (dir) {
// Here we can't use the hashes in the webpack chunks.
// That's because the "app.js" is not tight with a chunk.
Copy link
Member

Choose a reason for hiding this comment

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

is not tight with a chunk.

is not tied to a chunk is more correct here right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

@@ -0,0 +1,26 @@
// This pulgin combines a set of assets into a single asset
Copy link
Member

Choose a reason for hiding this comment

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

pulgin => plugin

@arunoda arunoda merged commit 32af829 into vercel:master Mar 24, 2017
@arunoda arunoda deleted the async-js-load branch March 24, 2017 07:51
@matthewmueller
Copy link
Contributor

should we also defer this bundle?

@arunoda
Copy link
Contributor Author

arunoda commented Apr 7, 2017

should we also defer this bundle?
No. Defer and Async are different stuff. We might use that for backward compatibility.
But since we only focus on IE9+, this is fine.

@tomevans18
Copy link

I feel like this topic has been done to death but I wanted to comment and make sure I'm on the right lines and nothing has been missed.

After trying to improve the performance of a site I'm working on I came across this PR.
On review of Chrome's performance profiler, I noticed that the main.js file is taking a long time to evaluate (1.6 seconds in my case) and this seems to be slowing down the sites first meaningful paint.

My initial thought is deferring this file (along with all other JS) as it is not needed initially due to SSR.

image

What are your thoughts? Maybe all JS should be loaded after the DOM on initial load.
Is this achievable?

@timneutkens
Copy link
Member

timneutkens commented Jul 30, 2018

The long evaluation is probably fixed by #4639 as main.js used to serve as main bundle + commons but is now split.

In general, it makes not much sense to comment on a 1-year-old pull request to ask for help.

@tomevans18
Copy link

@timneutkens thanks for the info regarding #4639, I will keep an eye on that PR.

Apologies, I understood it was an old PR hence cavitating the comment with "I feel like this topic has been done to death". I could see that there were several comments on an issue (#1436) and this PR was to resolve it, with no future issues or PRs referencing this problem... hence the comment above.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants