sourceNodes TS types inconsistent with documentation usage

[JakubKoralewski]

Summary

In the documentation of sourceNodes API
the following function signature is used:

exports.sourceNodes = ({ actions, createNodeId, createContentDigest }) => {

But in the actual TypeScript type definition inside index.d.ts there are only the following:

  sourceNodes?(args: SourceNodesArgs, options: PluginOptions): any
  sourceNodes?(args: SourceNodesArgs, options: PluginOptions): Promise<any>
  sourceNodes?(
    args: SourceNodesArgs,
    options: PluginOptions,
    callback: PluginCallback
  ): void

This does not make sense, as the signature used in the docs does not exist in the type definition.

Motivation

TypeScript users will benefit

Steps to resolve this issue

Well the types should be updated, I didn't look into the actual code, but if it works like in the documentation then types should be updated.

Draft the doc

Open a pull request

• Open a pull request with your work including the words "closes #[this issue's number]" in the pull request description

[Js-Brecht]

I can't say I've run into any problems using the TS types.

The APIs that Gatsby passes into sourceNodes (and every other gatsby-* endpoint) are all part of an object. So this would work, too:

export const sourceNodes = (args: SourceNodesArgs) => {
   const { actions, createNodeId, createContentDigest } = args;
}

The second parameter to the function is also the same as the other endpoints. It contains the contents of the options property that is collected from gatsby-config for that particular plugin. If you are using sourceNodes in your root project, or if you don't define options for a certain plugin, then that parameter will only be a (mostly) empty object.

I say "mostly" because the plugins property will be tacked onto the object, but it will only be an empty array if you haven't defined any sub-plugins in gatsby-config.

So, basically, the type SourceNodesArgs is an object containing all of the various Gatsby API that are provided to that endpoint. Which includes actions, createNodeId, and createContentDigest.

[JakubKoralewski]

Yes, that's right! I ended up doing the same, although what I meant by this whole issue was to be able to just set the type on the function not on its parameters:

import { GatsbyNode } from "gatsby"
export const sourceNodes: GatsbyNode['sourceNodes'] = (args) => {}

Sorry if I didn't make this clear.

[Js-Brecht]

Oh, I see. Yeah, you're right. That has been an issue.

I'm about 99% sure it's unable to infer the argument types because of the function signature ambiguity. There's not enough for Typescript to differentiate between the various overloads.

Regarding the types for sourceNodes:

gatsby/packages/gatsby/index.d.ts

Lines 418 to 424 in 8ee74c1

	sourceNodes?(args: SourceNodesArgs, options: PluginOptions): any
	sourceNodes?(args: SourceNodesArgs, options: PluginOptions): Promise<any>
	sourceNodes?(
	args: SourceNodesArgs,
	options: PluginOptions,
	callback: PluginCallback
	): void

The third type in this list is able to be differentiated by it's third parameter, so we can just ignore it for now. It is important, though, because it forces the first two types to be much looser in their types, in order for them all to fit together.

As you can see, there's not a lot separating the first two except the return type. Even though all the parameter types are the same between calls, since Typescript cannot tell which overload to use, it just assigns any to them.

This is a poor way to type this kind of function call, anyway, since you would have to write the return value of the function before you can get the correct types on the function parameters. Even then, since the very first return type is any, and any can also mean Promise<any>, Typescript still doesn't know which overload to use.

You can't get rid of the first type, because that is a valid value... the return type can be a promise or not, and it is very useful for people to know what functions can be asynchronous. It also doesn't help to make it a union type, any | Promise<any>, because the same problems exist.

All of this together basically means a discriminated union is a better choice. It actually helps Typescript figure out what function type to use, so it is able to infer the parameter types.

This works for me:

interface GatsbyNodeAsyncFn<T extends NodePluginArgs, R> {
    (args: T): R | Promise<R>;
    (args: T, options: PluginOptions): R | Promise<R>;
}
interface GatsbyNodeCallbackFn<T extends NodePluginArgs> {
    (
        args: T,
        options: PluginOptions,
        callback: PluginCallback
    ): void
}

type GatsbyNodeAsync<T extends NodePluginArgs, R> = GatsbyNodeAsyncFn<T, R> | GatsbyNodeCallbackFn<T>;

export interface GatsbyNode {
    sourceNodes?: GatsbyNodeAsync<SourceNodesArgs, any>;
}

Realistically, the same/similar basic type can be assigned to the rest of the GatsbyNode API, because they all are able to return a Promise, or use a Callback. However, a lot of them are typed to return void, and not Promise<void>, and I assume that's because the intended use of those endpoints is to be synchronous. What's amusing about that, though, is that the types are obviously out of date. For example, createPages() can use the graphql function, which is asynchronous; but its types indicate that it is not an asynchronous function!

Because of the... intended... synchronous nature of many of the endpoints, it might make sense to include types like this:

interface GatsbyNodeSyncFn<T extends NodePluginArgs, R> {
    (args: T): R;
    (args: T, options: PluginOptions): R;
}
interface GatsbyNodeAsyncFn<T extends NodePluginArgs, R> {
    (args: T): R | Promise<R>;
    (args: T, options: PluginOptions): R | Promise<R>;
}
interface GatsbyNodeCallbackFn<T extends NodePluginArgs> {
    (
        args: T,
        options: PluginOptions,
        callback: PluginCallback
    ): void
}

type GatsbyNodeSync<T extends NodePluginArgs, R> = GatsbyNodeSync<T, R> | GatsbyNodeCallbackFn<T>;
type GatsbyNodeAsync<T extends NodePluginArgs, R> = GatsbyNodeAsyncFn<T, R> | GatsbyNodeCallbackFn<T>;

It's possible to coalesce those types, too, in order to avoid having to add GatsbyNodeCallbackFn to each union, but that may be adding too much complexity.

Sometimes it's also useful for the export you're defining to not be typed as optional. After all, you are assigning the value... why is it optional? This is an edge case though, and I've only run into it maybe once, myself. In any case, that could be fixed with a simple utility type:

export type GatsbyNodeFn<T extends keyof GatsbyNode> = Required<GatsbyNode>[T];

---

If you wanted to file a PR to fix this, it would be really appreciated. Personally, I haven't had the time or been concerned enough about it to go fix it. If you don't want to, I'm sure that's fine too. I, or somebody else, will probably get around to it eventually.

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

[flappyBug]

 sourceNodes?(args: SourceNodesArgs, options: PluginOptions): any 
 sourceNodes?(args: SourceNodesArgs, options: PluginOptions): Promise<any> 

As you can see, there's not a lot separating the first two except the return type. Even though all the parameter types are the same between calls, since Typescript cannot tell which overload to use, it just assigns any to them.

@Js-Brecht Unfortunately the actuary type of the function overload above is not any but something match both any and Promise<any>, thus Promise<any> (as of typescript 4.2). We shouldn't overload function with same parameters. It's an undefined behavior. For the above example, only return type of Promise<any> will pass the type checker, while in the below example, all string will pass the type checker.

function foo(): 'foo' | 'bar';
function foo(): 'bar' | baz';
function foo() {
    return 'any string will be fine';
}

Realistically, the same/similar basic type can be assigned to the rest of the GatsbyNode API, because they all are able to return a Promise, or use a Callback. However, a lot of them are typed to return void, and not Promise, and I assume that's because the intended use of those endpoints is to be synchronous.

void is generally used as return type of functions we don't care the result. It's officially recommended by typescript over return any. It not restrict the function to be sync. So it's fine to use as return type for createPages.

If we care the return value of sourceNodes, and will use that in somewhere else but can't determine the type, just use any, without an overload of Promise<any>. If we don't care, just annotate sourceNodes returns void. If you don't mind, I can make a pr for this tiny fix.

[Js-Brecht]

I think Promise<void> is better, primarily to signal to the consumer that they can use the function asynchronously. I can't think of any Gatsby Node API that actually wants a return value, and IIRC they all await the return

EDIT: There are a few gatsby-node api endpoints that want a return value. Just had to go look them up.