Gatsby: Remove flow annotations

[sidharthachatterjee]

We don't really use them and there are very few sprinkled through the code base.

[crutchcorn]

I'll take this one if that's okay with everyone

[sidharthachatterjee]

Sure @crutchcorn Thank you for picking it up! 🙌

[stefanprobst]

Can we somehow rescue the scattered types from type builders, field extensions and node model and put them in TS?

[sidharthachatterjee]

@stefanprobst Could definitely do that but I reckon TS will need more repo wide tooling setup? 😞 Or do you propose .d.ts files? That would be good I think

[stefanprobst]

yes, just .d.ts files

[crutchcorn]

I'll make sure to include the types in .d.ts files

[crutchcorn]

Do we also want to do JSDoc types additional to .d.ts definition files?

For example, something like this:

type QueryJob = {
  id: string,
  hash?: string,
  query: string,
  componentPath: string,
  context: Object,
  isPage: Boolean,
}

// Run query
module.exports = async (queryJob: QueryJob) => {

Could turn into something like this:

/**
 * @typedef QueryJob
 * @param {string} id
 * @param {string} [hash]
 * @param {string} query
 * @param {string} componentPath
 * @param {Object} context
 * @param {Boolean} isPage
 */

// Run query
/**
 * @param {QueryJob} queryJob
 */
module.exports = async queryJob => {

[sidharthachatterjee]

Probably best to not duplicate them across both. Since that’s one more thing that’ll need to be kept consistent. I think typescript definitions are a good source of truth and sufficient.

[crutchcorn]

I agree. I just was wondering your thoughts on this.

[crutchcorn]

Just wanted to jump in and mention that I have some initial work on this branch:

https://github.com/crutchcorn/gatsby/tree/remove-flow-definitions

But it seems to be causing various test failures. I intend to be looking into them today and tomorrow

[crutchcorn]

This PR has been opened and should be good-to-go (once merge conflicts are handled)

That said, it did introduce some broader questions raised in this comment:

https://github.com/gatsbyjs/gatsby/pull/16952#issuecomment-524958035

I'd love to get this resolved relatively soon so that merge conflicts don't grow to too high of scale

[brolag]

I'm interested in checking those conflicts

[gatsbot[bot]]

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

[gatsbot[bot]]

Hey again!

It’s been 30 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.

Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to reopen this issue or create a new one if you need anything else.

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 again for being part of the Gatsby community!