EnsureResources, doesn't ensure resources

[jquense]

Description

https://github.com/gatsbyjs/gatsby/blob/de3f32375da061977513e15b5e81cb2b4628135b/packages/gatsby/cache-dir/ensure-resources.js

SCU is called on updates, not on Mount, so if there are no resources, initially they are never fetched.

Steps to reproduce

Unclear how you set up the case where there are no resources, but it seems to be when pages are in status: "error"

Expected result

it should do work in componentDidMount to kick off updates if needed. Or use a hook

Actual result

What happened.

Environment

System:
OS: macOS Mojave 10.14.6
CPU: (12) x64 Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz
Shell: 5.7.1 - /usr/local/bin/zsh
Binaries:
Node: 12.13.0 - /var/folders/4c/08vnt0sx6zg3djdt5ln5rjk00000gp/T/yarn--1575565990280-0.563851784977069/node
Yarn: 1.19.1 - /var/folders/4c/08vnt0sx6zg3djdt5ln5rjk00000gp/T/yarn--1575565990280-0.563851784977069/yarn
npm: 6.12.0 - ~/.nvm/versions/node/v12.13.0/bin/npm
Languages:
Python: 2.7.16 - /Users/jquense/.pyenv/shims/python
Browsers:
Chrome: 78.0.3904.108
Firefox: 69.0.1
Safari: 13.0.3

[github-actions]

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! 💪💜

[jquense]

Up to the team if this is stale, but do want to make sure that it was seen. The bug is fairly clear (even if a repro isn't) when you look at the code.

[blainekasten]

@jquense thanks for the bug report! This is absolutely not stale and a real bug. We are currently a bit overloaded with post-holiday catch up. Is this something you're interested in submitting a PR? If you don't have the capacity for it we'll try to get to this in the next few weeks.

Seems like the PR could be relatively straight-forward:

1. Add the ensure resource logic to the cDM
2. Add some tests to ensure logic is called on mount

Or use a hook

Gatsby still supports early versions of React, so we are unable to use hooks in our runtime components as of yet.

Let me know what if you'll be able to get to this or not!

[jquense]

I'd be happy to write the PR if i get a second, but it'd be unlikely i'd have the correct context or codebase knowledge to write an effective test. At the moment there is really no rush on fixes this as (AFAICT) it practically only shows up to cover a real error, e.g. missing page info.

[blainekasten]

@jquense I was looking into this and I'm not certain I see the issue. Did you run into this or did you just identify it while looking through the code?

From what I see, we initialize pageResources in the constructor by synchronously loading the resources for a given path. If we implemented a componentDidMount check, it would fail in the same way that the initial state setting would fail if there is actually indeed a problem.

gatsby/packages/gatsby/cache-dir/ensure-resources.js

Line 11 in de3f323

pageResources: pageResources || loader.loadPageSync(location.pathname),

[jquense]

I did encounter this when pages were in a bad state, e.g. everything built and compiled but page data was missing or in an error state. The error this throws is really confusing, It could totally be the case that the problem here is that you actually can't ensure page resources, and this should fail more gracefully

[blainekasten]

I see. That helps to have that knowledge. I’ll work on this some more and figure out if that’s the problem and we can improve messaging.

[jquense]

@blainekasten FYI i can reproduce this fairly easily one a gatsby site with no pages. e.g. empty pages directory. which is a confusing error