context.nodeModel.runQuery should return [] instead of null

[aaronadamsCA]

Description

With a query that returns no results, context.nodeModel.runQuery returns Promise<null>, but the documentation specifies non-null type Promise<Node[]>.

I'd prefer to get back [] instead of null, because dealing with one type is easier than dealing with two, and because it's consistent with how the rest of Gatsby GraphQL works. If I pass the same filter to a static query and runQuery, I think both should return the same result, [].

Environment

  System:
    OS: Linux 4.19 Debian GNU/Linux 10 (buster) 10 (buster)
    CPU: (12) x64 Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz
    Shell: 5.0.3 - /bin/bash
  Binaries:
    Node: 12.16.3 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/bin/yarn
    npm: 6.14.4 - /usr/local/bin/npm
  Languages:
    Python: 2.7.16 - /usr/bin/python
  npmPackages:
    gatsby: ^2.24.4 => 2.24.4
    gatsby-image: ^2.4.13 => 2.4.13
    gatsby-plugin-google-gtag: ^2.1.10 => 2.1.10
    gatsby-plugin-manifest: ^2.4.18 => 2.4.18
    gatsby-plugin-netlify: ^2.3.11 => 2.3.11
    gatsby-plugin-react-helmet: ^3.3.10 => 3.3.10
    gatsby-plugin-sharp: ^2.6.19 => 2.6.19
    gatsby-plugin-theme-ui: ^0.3.0 => 0.3.0
    gatsby-source-sanity: ^6.0.3 => 6.0.3
    gatsby-theme-style-guide: ^0.3.1 => 0.3.1
    gatsby-transformer-sharp: ^2.5.11 => 2.5.11
  npmGlobalPackages:
    gatsby-cli: 2.12.45

[vladar]

Hi @aaronadamsCA !

Returning an empty array sounds correct to me too. But we explicitly test for null for some reason:

gatsby/packages/gatsby/src/redux/__tests__/run-fast-filters.js

Lines 189 to 190 in cab7155

	expect(resultSingular).toEqual([])
	expect(resultMany).toEqual(null)

The test was added long ago in #11448 and the implementation was there even longer.

So, my main concern is that it is a breaking change. I think we should still change this behavior, but the question is if we should wait for the next major or can include it as a patch/minor. @pieh @pvdz any thoughts on this or maybe more context?

Anyways, the PR is welcome if you're up to it!

I think the change should be trivial somewhere here (just always return an empty array):

gatsby/packages/gatsby/src/redux/run-fast-filters.ts

Lines 419 to 424 in cab7155

	if (firstOnly) {
	return []
	}
	return null
	}

[kartikcho]

Can I provide a patch with these changes above @vladar, after it's confirmed and if it's alright with @aaronadamsCA?

[aaronadamsCA]

@kartikcho Absolutely, I just didn't know where to start - though it looks like @vladar already found the spot. 👍

@vladar My guess is somebody just got the tests backwards? Because I'd probably expect null if I'd asked for an object vs. [] if I'd asked for an array. I'm guessing not toooo many people get this deep into the APIs.

[pvdz]

(As mentioned in the PR, this is a subtle breaking change that I'd love to apply but will have to be done in a major bump because it's bound to break somebody's setup)

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