-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
gatsby/cache-dir/loader.js
eval's JSON on main thread, doesn't use fetch()
#18787
Comments
(Note: I’m not a Gatsby maintainer, just saw this ticket and I’m curious) My understanding has been that using Response's Testing right now in latest Chrome seems to confirm this is still the case, though I don't have time to put together an example that is easy to understand, here is at least an example: MEGA HUGE WARNING PLEASE READ THIS FIRST: THIS LOADS 135 MB OF JSON YOU HAVE BEEN WARNED: https://jsbin.com/fizokeq/1/edit?js,output It's always possible what I'm seeing is not parse times, but something else though? I haven’t yet looked to see if the spec would allow off thread parsing, but seemingly wouldn’t be observable (ignoring that “it resolves faster”) |
Reason for not using
and later on doing Related links:
It seems like now the issue in chrome is fixed now - there are no more double requests when using Thanks for bringing this issue up! |
|
Maybe we should just switch to |
I've been doing some research on that and seems like using
I wasn't really comparing performance of those methods (too few runs for that), but it seems that none of them is great. We could potentially look into solutions like https://www.npmjs.com/package/bfj that yields control periodically allowing event loop to be handled, but I think better approach would be to make sure that user don't end up with those huge json files instead, because that's bigger issue. |
Debugging chromium itself seems to confirm that
So I would say using XHR (no need to polyfill it for ie) + Any thoughts on that part? While it seems we can't really attack json parsing perf here, the bigger problem is that this site ended up with this massive json file, and we should look into implementing some perf budgets solutions into build process. |
Hiya! This issue has gone quiet. Spooky quiet. 👻 We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open! As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing! Thanks for being a part of the Gatsby community! 💪💜 |
I'll close this issue in favour of #19780 which is more actionable than this one. |
Description
In traces like this one:
https://www.webpagetest.org/result/191017_J5_d438a3f9ef614a319b46a66ae936fb5b/1/details/#waterfall_view_step1
The way that Gatsby's
loader.js
evaluates JSON leads to long main-thread tasks. At core, this is because of the use ofJSON.parse(responseText)
from an XHR.Use of the
fetch()
method would allow asynchronous and off-thread JSON decoding that would not block user interaction.Steps to reproduce
See trace linked above.
Expected result
Large JSON files should not block the main thread.
Actual result
Large JSON files block the main thread when loaded.
The text was updated successfully, but these errors were encountered: