Skip to content

Commit

Permalink
Load script with the defer attribute (#1437)
Browse files Browse the repository at this point in the history 
Load script with the defer attribute
  • Loading branch information
@arunoda
arunoda authored Mar 19, 2017
1 parent ffade8d commit b2fabf0
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion server/document.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export class NextScript extends Component {
const hash = buildStats ? buildStats[filename].hash : '-'

return (
<script type='text/javascript' src={`/_next/${hash}/${filename}`} />
<script type='text/javascript' src={`/_next/${hash}/${filename}`} defer />
)
}

Expand Down

8 comments on commit b2fabf0

@ptomasroos
Copy link
Contributor

Choose a reason for hiding this comment

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

Note. this will defer the parsing but not make the window onload faster. It will not release the spinning circle of the browser but let other scripts which is not deffer get "higher" priority to parse before.

At least my understanding of it @arunoda, agree?

@arunoda
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is.
Having a different solution takes times and we should need to do it and test more.
See: #1436 (comment)

So, we took this for 2.0 release. It's a much better than the current case :)

@timneutkens
Copy link
Member

Choose a reason for hiding this comment

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

👌

@lukeed
Copy link
Contributor

@lukeed lukeed commented on b2fabf0 Mar 22, 2017

Choose a reason for hiding this comment

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

What if you guys load a single async script on the initial render. This can/would only contain your critical core bits, along with a location parser that fetches the necessary chunk(s) for the current page.

First paint would decreased, as well as payload size. I'm doing this is in a local app right now...

With the current examples/using-preact (adding compression and express):

  • DOMContentLoaded in 85ms
  • 39.4KB total transferred

With a direct clone of that example (using my async suggestion, but not Next.js):

  • DOMContentLoaded in 34ms
  • 7.8KB total transferred

In my first go-round, I had 2 scripts too, both with defer and my DOMContentLoaded hovered around 65ms. A single async was the way to go, at least for me.

@arunoda
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lukeed async might execute before the DOM render. So, we are tracking here about a much better options. See: #1436

I think providing a single script makes sense. I'll give it a try.

@lukeed
Copy link
Contributor

@lukeed lukeed commented on b2fabf0 Mar 22, 2017

Choose a reason for hiding this comment

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

@arunoda > async might execute before the DOM render

Shouldn't matter here. The __NEXT__ var is populated from the server, and there are no document.write instances that I can recall. But just by adding async, we unblock the render and let the (supported) browsers continue. Whenever the script loads, control of the DOM elements is handed off seamlessly, while the user has no idea. Aka, no screen flashes or jitter.

@arunoda
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lukeed yes. Anyway, we need to make find a way to merge our two files.
That's the first thing to do.
After that, we can do either way.
I think we should do this before 2.0

@arunoda
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lukeed implemented with: #1485

Please sign in to comment.