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.

Sign up for GitHub

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

Jump to bottom

[gatsby] v1.9.157 - TypeError: Cannot read property '35783957827783' of undefined when running in production mode... #3568

Closed
busticated opened this issue Jan 17, 2018 · 5 comments
Closed

[gatsby] v1.9.157 - TypeError: Cannot read property '35783957827783' of undefined when running in production mode... #3568

busticated opened this issue Jan 17, 2018 · 5 comments

Comments

@busticated
Copy link
Contributor

I ran into a weird error after upgrading to gatsby v1.9.157. if i manually revert this change, everything works as expected. likewise, downgrading to gatsby v1.9.155 fixes the issue.

Description

after running gatsby build && gatsby serve then visiting http://localhost:9000, i see the following error sporadically:

bootstrap 98fe446b590db3b33425:80 Uncaught (in promise) TypeError: Cannot read property '35783957827783' of undefined
    at t.e (bootstrap 98fe446b590db3b33425:80)
    at Function.n.e (patch.js:28)
    at t.exports (index.js?6af5:4)
    at N (loader.js:85)
    at k (loader.js:112)
    at Object.getResourcesForPathname (loader.js:358)
    at production-app.js:144
    at <anonymous>

it appears in production mode only and seems to be related to resource loading. it's sporadic but happens on initial page load (roughly 50% of the time). i think the faster the response, the more likely it is that you'll hit the error. the file with the error has the url:

webpack:///webpack/bootstrap 98fe446b590db3b33425

35783957827783 referenced in the error message is the chunkId used in this line within the above mentioned file:

script.src = __webpack_require__.p + window["webpackManifest"][chunkId];

Steps to reproduce

  1. run gatsby build && gatsby serve
  2. load http://localhost:9000/ in your browser
  3. refresh w/ dev console open until you see the error

Environment

Gatsby version: 1.9.157
Node.js version: 8.9.4
Operating System: macOS 10.13.2
@busticated
Copy link
Contributor Author

cc @szimek

@szimek
Copy link
Contributor

szimek commented Jan 17, 2018 •
edited
Loading

😭 I checked it multiple times and it always worked for me... Could you check what exactly triggers this line:

script.src = __webpack_require__.p + window["webpackManifest"][chunkId];

?

According to the docs, inline scripts should be loaded synchronously and delay DOMContentLoaded event, which Gatsby waits for before it starts rendering React app. So I thought that any JS code that depends on windows.webpackManifest to be present will be loaded after it, regardless if it's in <head> or at the end of <body>.

@szimek
Copy link
Contributor

szimek commented Jan 17, 2018 •
edited
Loading

I suspect it's caused by this script loader https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/cache-dir/static-entry.js#L186-L196.

Gatsby preloads all scripts using <link rel="preload" />, but this doesn't execute them, so it can't cause this issue. However, it also adds this script loader to the bottom of <head>, which loads the same scripts that are preloaded at the top of <head>.

Maybe it's possible that if everything is loaded super fast, this loading script will load all scripts before the browser parses the script tag with window.webpackManifest at the end of the body.

@busticated Could you change this line in your node_modules/gatsby from:

headComponents.push(

to

postBodyComponents.push(

so that it's after the script tag with webpack manifest (which is added using postBodyComponents.unshift), rebuild the app and see if this issue persists?

@busticated
Copy link
Contributor Author

@szimek thanks for taking a look 👍

using gatsby v1.9.157 and after making the change you suggested, it seems like things are working fine. i'm no longer able to reproduce the error i originally reported.

@szimek szimek mentioned this issue Jan 17, 2018
Merged
@KyleAMathews
Copy link
Contributor

Ok, this was the problem that I couldn't remember 😅

So yeah, the webpack manifest sets webpackManifest on the window object. Then each of the webpack bundles use that to translate their unique ids (e.g. commons-68715c66e10bd35fbd81.js) back to commons.js which is what we refer to it internally.

So if scripts are loaded in the head and the manifest isn't until the end then we run the risk (increased the longer the HTML body is) of a script loading, expecting webpackManifest to be on the window, before the manifest is read setting webpackManifest.

We should read the manifest then load scripts which is what it was before moving the manifest to after the body and what @szimek's PR in #3569 restored.

There's two primary metrics we're optimizing for. TTFP (time to first paint) and TTI (time to interactivity).

Loading the manifest and starting the scripts downloading (async so non-blocking) optimized TTI while slowing TTFP somewhat especially for sites with really large numbers of files in the manifest as all that JS has to be read & parsed before the browser can get to the body.

Putting the manifest & scripts at the end hurts TTI as now the HTML has to be fully loaded & parsed before we can start being interactive. It's especially slow for the ~50% of browsers that don't support preloading scripts as we don't even start downloading scripts until the end of the body.

Now for smaller pages it's basically the same either way. But there are plenty of sites with HTML documents > 100kb and on poorer connections, these differences start to really matter.

In the future, I've been thinking we'll move query results out of webpack which will remove most of the webpack manifest bloat. For a site with a ton of pages, most of the manifest is made up of pointers to individual page json files — most of which are never needed. See this other issue I just put up for that #3574

Once we do that, let's move the manifest + script loading back to the head because even for very large sites, the # of page components won't be that large.

Thanks @busticated and @szimek for your help here!

@KyleAMathews KyleAMathews mentioned this issue Jan 18, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@szimek @KyleAMathews @busticated