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.

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

chore(gatsby): add TypeScript definitions for config & plugin APIs #10897

Merged
merged 8 commits into from
Apr 30, 2019

Conversation

JamesMessinger
Copy link
Contributor

Description

I added TypeScript definitions for the gatsby-config.js, gatsby-node.js, gatsby-browser.js, and gatsby-ssr.js APIs.

Some of the definitions could use more details (for example, many functions are simply defined as Function), but at least every method, parameter, and property are defined, which provides good intellisense and code-completion assistance in tools like VSCode. 😎

Related Issues

N/A

I created TypeScript definitions for the [`gatsby-config.js`](https://www.gatsbyjs.org/docs/gatsby-config/#gatsby-config), [`gatsby-node.js`](https://www.gatsbyjs.org/docs/node-apis/), [`gatsby-browser.js`](https://www.gatsbyjs.org/docs/browser-apis/), and [`gatsby-ssr.js`](https://www.gatsbyjs.org/docs/ssr-apis/) APIs. 

Some of the definitions could use more details (for example, many functions are simply defined as `Function`), but at least every method, parameter, and property are defined, which provides good intellisense and code-completion assistance in tools like VSCode. 😎
@JamesMessinger JamesMessinger requested a review from a team as a code owner January 7, 2019 20:27
@pieh
Copy link
Contributor

pieh commented Jan 8, 2019

That's really cool! Users would need to annotate types of the exports in gatsby-X files, right? How does it look like?

I was working on something similar (but using jsdoc) and I did hit lot of problems but made it to somewhat work:
screenshot 2019-01-06 at 17 22 52

Reason for using jsdoc instead of TS definition was that we generate API reference ( https://www.gatsbyjs.org/docs/node-apis/ ) from that, so we wouldn't need to maintain definitions in multiple places (typescript defs could not always be up to date)

@JamesMessinger
Copy link
Contributor Author

JamesMessinger commented Jan 8, 2019

@pieh - Sorry, I should have included a usage example in my original description. VSCode (and other TypeScript-aware editors) support TypeScript definitions in plain JavaScript files (like gatsby-config.js) via a combination of JSDoc and "magic comments", such as:

  • /// <reference types="..." /> (docs)
  • // @ts-check (docs)

For example, here's what my gatsby-config.js file looks like:

// @ts-check
const gatsby = require("gatsby");

/**
 * @type {gatsby.GatsbyConfig}
 */
const config = {
  siteMetadata: {
    title: "Welcome to my website!",
  },

  plugins: [
    "gatsby-plugin-typescript",
    {
      resolve: "gatsby-source-filesystem",
      options: {
        name: "docs",
        path: `${__dirname}/src/docs/`,
      },
    },
  ],
};

module.exports = config;

Because I've declared my config variable to be of type gatsby.GatsbyConfig, I get code-completion, type-checking, and intellisense in VSCode. Which is really nice!

@eknowles
Copy link
Contributor

thank you for this 👍

@JamesMessinger
Copy link
Contributor Author

Oh wait! Before you merge this!

I hadn't heard any feedback on this PR, so I thought it wasn't going to be merged. So I was preparing to submit type definitions to DefinitelyTyped instead. But it's always preferrable to have type definitions in the library package itself, so I'm glad to see that you're interested.

Here are the latest type definitions (attched ZIP file), which I was preparing to submit to DefinitelyTyped. I've done a lot of cleanup, including separating the definitions into multiple files and correcting a few mistakes in my original definitions. In addition, I've added types for some Gatsby plugins too.

gatsby-types.zip

  • gatsby-plugins
    This folder has the type definitions for the Gatsby plugin APIs (gatsby-browser, gatsby-config, gatsby-node, and gatsby-ssr)

  • gatsby-source-filesystem
    This folder has the type definitions for the gatsby-source-filesystem plugin

  • gatsby-mdx
    This folder has the type definitions for the gatsby-mdx plugin

@LekoArts
Copy link
Contributor

LekoArts commented Apr 4, 2019

Hi @JamesMessinger 👋
Would be great to have this in. Is that what you zipped up also in this PR? And could you also please have a look at the merge conflict? Thanks!

@LekoArts LekoArts added the status: awaiting author response Additional information has been requested from the author label Apr 4, 2019
@KyleAMathews
Copy link
Contributor

This is great stuff! Would love to have it merged!

@orta
Copy link
Contributor

orta commented Apr 25, 2019

I've tried to merge in the zip changes, and add my own API reference JSDoc comments based on this PR in #13619

@wardpeet wardpeet self-assigned this Apr 26, 2019
@wardpeet wardpeet changed the title TypeScript definitions for config & plugin APIs chore(gatsby): add TypeScript definitions for config & plugin APIs Apr 26, 2019
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm updating the interfaces as there were some issues when trying to use it. (give me a sec to wrap up ^^)

for example this wasn't working:

/** @typedef {import('gatsby').GatsbyNode} GatsbyNode */

/** @type {GatsbyNode} */
const typedExports = exports

typedExports.createPages = (args, plugins) => { }

I might be just missing a lot of ts knowledge.

packages/gatsby/index.d.ts Show resolved Hide resolved
packages/gatsby/index.d.ts Show resolved Hide resolved
@wardpeet
Copy link
Contributor

wardpeet commented Apr 26, 2019

@orta maybe you know how to solve this.

Typings are correct as you can see below:
image

There is a problem as typescript can't detect the types of the params as it's a union of functions. So args & plugins are treat as an any value (if I remove the union and put only 1 function inside the d.ts file it works)

image
image

With only one function in d.ts file
image
image

@orta
Copy link
Contributor

orta commented Apr 26, 2019

@wardpeet my PR is the successor to this #13619 - can you move any improvements there? I already fixed the above

@wardpeet
Copy link
Contributor

sweet let me rollback my changes.

Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet, thanks for doing the hard work of creating types for our apis so much appreciated!

@wardpeet wardpeet merged commit 68ec040 into gatsbyjs:master Apr 30, 2019
@gatsbot
Copy link

gatsbot bot commented Apr 30, 2019

Holy buckets, @JamesMessinger — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

  1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. We’ve got Gatsby t-shirts, stickers, hats, scrunchies, and much more. (You can also unlock even more free swag with 5 contributions — wink wink nudge nudge.) See gatsby.dev/swag for details.
  2. We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. Accept the invite by visiting https://github.com/orgs/gatsbyjs/invitation. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!

@wardpeet
Copy link
Contributor

published in gatsby@2.3.34

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting author response Additional information has been requested from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants