[TS] TypeScript errors updating gatsby 2.3.33 to 2.3.34

[wKovacs64]

Description

Upon attempting to update the core gatsby package from 2.3.33 to 2.3.34, I ran into a few TypeScript errors introduced in #13619:

node_modules/gatsby/index.d.ts(3,29): error TS7016: Could not find a declaration file for module 'express'. 'gatsby-2334-ts/node_modules/express/index.js' implicitly has an 'any' type.
  Try `npm install @types/express` if it exists or add a new declaration (.d.ts) file containing `declare module 'express';`
node_modules/gatsby/index.d.ts(812,15): error TS2300: Duplicate identifier 'Node'.
node_modules/gatsby/index.d.ts(812,15): error TS7031: Binding element 'Node' implicitly has an 'any' type.
node_modules/gatsby/index.d.ts(812,28): error TS2300: Duplicate identifier 'Node'.
node_modules/gatsby/index.d.ts(812,28): error TS7031: Binding element 'Node' implicitly has an 'any' type.
node_modules/gatsby/index.d.ts(869,3): error TS7010: 'addThirdPartySchema', which lacks return-type annotation, implicitly has an 'any' return type.
node_modules/gatsby/index.d.ts(872,5): error TS1016: A required parameter cannot follow an optional parameter.
node_modules/gatsby/index.d.ts(876,3): error TS7010: 'createTypes', which lacks return-type annotation, implicitly has an 'any' return type.
node_modules/gatsby/index.d.ts(879,5): error TS1016: A required parameter cannot follow an optional parameter.

1. We need @types/express now. Easy enough to add to my project, but maybe it should be a Gatsby dep/devDep? Not sure.
2. Gatsby's Node interface conflicts with Node from the TypeScript dom library. Not sure how to handle this, maybe the declarations just need wrapped in a namespace? @orta or someone will probably know the right:tm: solution. (resolved with point 5 below)
3. addThirdPartySchema and createTypes just need their return types specified (looks like void - easy PR, but wanted to figure out these other errors as well).
4. addThirdPartySchema and createTypes declarations currently flag the plugin parameter as optional and the traceId parameter as required, but it looks like in reality plugin is required and traceId is optional (in both cases)? Another easy fix if I'm understanding it correctly.
5. createParentChildLink signature is slightly incorrect (renaming while destructuring vs. typing).

(Side note: "skipLibCheck": true will silence the errors, but that's just a hack.)

Steps to reproduce

Create a Gatsby+TS project with "strict": true in tsconfig.json and check the types with tsc.

Expected result

Type checking should pass.

Actual result

Type checking fails.

Environment

System:
  OS: Windows 10
  CPU: x64 Intel(R) Core(TM) i7-7700 CPU @ 3.60GHz
Binaries:
  Yarn: 1.15.2 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
  npm: 6.9.0 - C:\Program Files\nodejs\npm.CMD
Languages:
  Python: 2.7.15 - /c/Users/user/.windows-build-tools/python27/python
Browsers:
  Edge: 41.16299.1004.0
npmPackages:
  gatsby: 2.3.34 => 2.3.34 
  gatsby-image: 2.0.41 => 2.0.41 
  gatsby-plugin-manifest: 2.0.29 => 2.0.29 
  gatsby-plugin-offline: 2.0.25 => 2.0.25 
  gatsby-plugin-react-helmet: 3.0.12 => 3.0.12 
  gatsby-plugin-sharp: 2.0.35 => 2.0.35 
  gatsby-source-filesystem: 2.0.33 => 2.0.33 
  gatsby-transformer-sharp: 2.1.18 => 2.1.18 

[orta]

We need @types/express now. Easy enough to add to my project, but maybe it should be a Gatsby dep/devDep? Not sure.

IMO this should probably get removed from exported types (and switched with an any) - the lack of a dependency graph for @types is annoying.

Duplicate identifier 'Node'.

Yeah, I think Gatsby will need to re-name Node here. There was a GatsbyNode - but I feel like that does something else?

For the rest, you have more strict mode options turned on than my project did. We need to get a linter on the dts at some point, not sure if I'll have much time until the weekend to look into more of this, so it might be wise to ship a PR yourself 👍

[wKovacs64]

Yeah, I think Gatsby will need to re-name Node here. There was a GatsbyNode - but I feel like that does something else?

Looks like GatsbyNode is for the gatsby-node.js API. How much damage would we do if we renamed that to GatsbyNodeAPI or something and renamed Node to GatsbyNode? 😝 (could also rename the other API interfaces for consistency)

I can probably PR everything but the linter shortly if that sounds acceptable.

[orta]

I think that makes sense - yeah!

[pieh]

[email protected] with fixes was published