fix(gatsby): update plugin api types

[Js-Brecht]

Description

This updates the public types exposed for plugin api endpoints so that Typescript is better able to resolve types of function signatures.

I opted for simplicity, removing the various function overloads that were causing issues.
I've also fixed the return types for certain endpoints, updated the signature for some endpoints that didn't seem accurate (GatsbySSR), and added some generic parameters that will allow more effective usage of some of the endpoints (onCreateNode, onCreatePage, etc).

• Each of the GatsbyNode apis can be characterized with all three parameters: args, options, and the callback. Whoever is writing the endpoint can choose which parameters they wish to consume.

  • If it's decided that this doesn't clearly document the usage of the api (e.g. can use callback or not), then an alternative is to do a union of overload interfaces. This will still allow Typescript to infer the types correctly, but the intellisense output is not as clear: you have to Go to definition and walk up the type definitions to see what the actual function signature is.

• In GatsbySSR, it appears as though they do not receive a callback parameter. Not only that, but I don't believe any of them can be asynchronous? Please correct me if I'm wrong

• There were several types that take generic parameters, but the way the types were designed made it impossible to actually use those generics. Anytime I've wanted to, I've had to recreate the function types myself. This adds those parameters to the interface (instead of on the property) and passes them down to the types from there.

• I also added a generic on createNode, so that you can make sure the nodes you're creating are strictly typed.

  • By doing this, I've changed InputNode to a type instead of an interface. This means that declaration merging won't have any affect on it now. I thought it was pretty safe to do, but perhaps there's people out there that have used declaration merging to type their nodes? It seems like it would be a strange thing to do for me, and it would be simple enough to switch over to using the generic parameter instead.
  • EDIT: this is too destructive so reverted to interface and used a union in createNode()

• I thought about adding a generic to PluginOptions too, so that plugin designers could make sure their plugin options are strongly typed. Since the types have been broken for some time, I've usually just recreated the function signatures in my projects, with their own strongly typed plugin options.

  • I didn't include the generic because this is somewhere I would definitely understand somebody doing declaration merging, especially because that would take affect across all endpoints, with no extra work/generics.
  • I would need to do some testing with the declaration merging for PluginOptions. I can't remember if third-party libraries will affect your own definitions if you import their module. For example, if I import foo library to use some function they've created, and they used declaration merging to define their PluginOptions, will it affect my own PluginOptions.

• The return types for various endpoints were rather ambiguous, when they really could have been more explicit.

Additional Context

There is a lot of eslint issues in index.d.ts. I'm not sure how much this matters... I had to fight my editor every step of the way in order to keep eslint turned on (I prefer using auto-fix, and don't like adjusting my settings for individual files) 😆. It also makes it harder to find legitimate issues

---

If we're being honest, the plugin api design makes it a little bit cumbersome to type & consume those types. I think it would be awesome to see a "plugin builder" api instead, which could be used like this:

import type { GatsbyNodePlugin } from "gatsby";

type MyCustomNode = {
  foo: string;
}

type MyOverrideNode = {
  bar: string;
}

// This will set the default node type created by this plugin to `MyCustomNode`
const plugin: GatsbyNodePlugin<MyCustomNode> = (builder) => {
  // `builder.sourceNodes` accepts a node type too, but falls back to `MyCustomNode`
  builder.sourceNodes<MyOverrideNode>(({ actions }) => {
    const { createNode } = actions;
    
    createNode({ ..., bar: "baz", ... }) // I'm strongly typed
  });
}

export default plugin;

This type of plugin creator structure makes types so much clearer and easier to work with. If it were to accept both an array of builders or the single builder method itself, it would also be really simple to break up your gatsby-node into multiple modules; just import them all into the gatsby-node and expose them as an array of function references.

Just some thoughts 🤷‍♂️

Related Issues

Closes #23296

[LekoArts]

Hi!
As always, your PRs are much appreciated!

Each of the GatsbyNode apis can be characterized with all three parameters: args, options, and the callback. Whoever is writing the endpoint can choose which parameters they wish to consume

Yep, sounds correct 👍 People can use any of those three but don't have to

In GatsbySSR, it appears as though they do not receive a callback parameter. Not only that, but I don't believe any of them can be asynchronous? Please correct me if I'm wrong

Via Git Blame I traced it back to #10897 and I think at that point in time we just accepted it as is, but I also don't see how they could be async

There were several types that take generic parameters, but the way the types were designed made it impossible to actually use those generics. Anytime I've wanted to, I've had to recreate the function types myself. This adds those parameters to the interface (instead of on the property) and passes them down to the types from there.

Yep, had similar experiences in the past

I thought about adding a generic to PluginOptions too, so that plugin designers could make sure their plugin options are strongly typed. Since the types have been broken for some time, I've usually just recreated the function signatures in my projects, with their own strongly typed plugin options.

Can you expand on that a bit, maybe with a code example? For now when I have my custom plugin options I create an interface that extends Gatsby's PluginOptions and manually type the plugin options arg. Would your proposal be that you only define it once and the args would know of it automatically (in all functions)?

The return types for various endpoints were rather ambiguous, when they really could have been more explicit.

👍

There is a lot of eslint issues in index.d.ts. I'm not sure how much this matters... I had to fight my editor every step of the way in order to keep eslint turned on

Porting such a huge codebase over to TS takes time and effort -- we really want that file to be autogenerated but we only got so many people to handle everything here 😅

If we're being honest, the plugin api design makes it a little bit cumbersome to type & consume those types. I think it would be awesome to see a "plugin builder" api instead, which could be used like this:

If you want you can take this to an RFC: https://github.com/gatsbyjs/gatsby/discussions/categories/rfc

[Js-Brecht]

Can you expand on that a bit, maybe with a code example? For now when I have my custom plugin options I create an interface that extends Gatsby's PluginOptions and manually type the plugin options arg. Would your proposal be that you only define it once and the args would know of it automatically (in all functions)?

Absolutely!

The method that could be used now would be declaration merging, but I only left it that way because I was trying to minimize the changes to the public types. It's a little bit esoteric and isn't very common for public APIs, but can't say it's never used... it may also cause some additional problems I mentioned when importing other people's plugins:

// foo-plugin/gatsby-node
import { GatsbyNode, PluginOptions } from "gatsby";

declare module "gatsby" {
  interface PluginOptions {
    foo: string;
  }
}

export const sourceNodes: GatsbyNode["sourceNodes"] = (args, options) => {
  const test: string = options.foo; // No errors
}

The reason I say there may be an issue with importing that plugin is if foo-plugin exports something from gatsby-node (or wherever they did the declaration mergin), and I import that into my own plugin:

// bar-plugin/gatsby-node

// Let's say `someUtility` is getting its types from the same module where the declaration merging was done
import { someUtility } from "foo-plugin";
import { PluginOptions, GatsbyNode } from "gatsby";

interface IMyPluginOptions extends PluginOptions {
  bar: string;
}

export const sourceNodes: GatsbyNode["sourceNodes"] = (args, options: IMyPluginOptions) => {
  const bar: string = options.bar; // Expected to be good

  // With `noUncheckedIndexedAccess`, I would expect an error here; but with the declaration
  // merging, this plugin might inherit the types of `foo-plugin`
  const test: string = options.foo;
}

I can't remember if Typescript will ignore declaration merging done in 3rd party libraries. Would need to test it.

---

Probably a better method (and arguably more common) would be to define the extended plugin options as generic on PluginOptions, and define it through the API interface. Problem with this is that PluginOptions needs to change from interface to type, which could potentially cause some problems where it is used. However, a way around that is to define it as a union on every API endpoint

// With generic parameter.  This has the added benefit of making them strictly typed when TExtendOptions is defined
export type PluginOptions<
  TExtendOptions extends Record<string, unknown> = Record<string, unknown>
> = {
  plugins: unknown[]
} & TExtendOptions;

export interface GatsbyNode<
  TPluginOptions extends Record<string, unknown> = Record<string, unknown>,
  TNode extends Record<string, unknown> = Record<string, unknown>,
  TContext = Record<string, unknown>
> {
  sourceNodes?(
    args: SourceNodesArgs,
    options: PluginOptions<TPluginOptions>, // IMO this is cleaner than using unions everywhere
    callback: PluginCallback<void>
  ): void | Promise<void>
}

// With a union instead
export interface PluginOptions {
  plugins: unknown[]
  [key: string]: unknown
}

export interface GatsbyNode<
  TPluginOptions extends Record<string, unknown> = Record<string, unknown>,
  TNode extends Record<string, unknown> = Record<string, unknown>,
  TContext = Record<string, unknown>
> {
  sourceNodes?(
    args: SourceNodesArgs,
    options: PluginOptions & TPluginOptions, // This would have to be done everywhere `PluginOptions` is used
    callback: PluginCallback<void>
  ): void | Promise<void>
}

Usage is the same with both definitions:

// Some plugin
import { GatsbyNode } from "gatsby";

interface IPluginOptions {
  foo: string;
}

// I'd probably create an alias
type MyGatsbyNode = GatsbyNode<IPluginOptions>;

/* Maybe if I wanted to allow it to define different kinds of nodes: */
// type MyGatsbyNode<TNode extends Record<string, unknown> = Record<string, unknown> = (
//   GatsbyNode<IPluginOptions, TNode>
// );

const sourceNodes: MyGatsbyNode["sourceNodes"] = (args, options) => {
  const test: string = options.foo; // Should pass
}

[Js-Brecht]

Porting such a huge codebase over to TS takes time and effort -- we really want that file to be autogenerated but we only got so many people to handle everything here

I was tempted to fix all the linting issues, but I have run into some small problems when converting things like object to Record<string, unknown> in the past (object is a bit more permissive than Record is). Some of the fixes also included changing things like the names of interfaces (breaking change), as well as changing some types (like PageProps) from type to interface (this can also be a breaking change, depending on how those types are used in other projects).

That would also make this PR a lot more difficult to read, because the number of changes would be... a lot 😆

I struggle to see how much of this can be autogenerated. I guess ideally there would be an interface where things like GatsbyNode are consumed (converting api-runner-node looks like it would require quite a bit of work), and then that type would just be exported from the index. This is one reason I'm a fan of exporting factory/builder functions for public api... the types are carried along with them automatically.

If you want you can take this to an RFC: https://github.com/gatsbyjs/gatsby/discussions/categories/rfc

Sure, I can do that probably this weekend. It would be so cool to get away from the magic exports, but once a project has come this far it's a struggle to change the public API so drastically. Wind up with this compatibility layer to support both for a while, and then all that extra work to strip that layer out after the old style has been successfully deprecated, not to mention all the extra maintenance for both and the in-between 😞.

I've been considering making this type of usage an option in gatsby-plugin-ts-config v2, which was actually the impetus behind fixing the types finally (need to be able to pass types/generics up & down). With the types more usable (especially as generics), I could probably make it work with Proxy and some callback chaining. It would be some fancy stuff, so mostly for my own benefit/entertainment 😂, but pretty cool overall

[LekoArts]

Thanks for the explanations on these things! I'll get this merged as-is (and then it'll be released 11th of May) and would still be really interested in a RFC write-up. We'll want to introduce a Strict Mode into Gatsby, maybe parts of the ideas could flow into this then.