chore(gatsby): type exposed workerpool functions #31768
Conversation
gatsbot
bot
added
the
status: triage needed
pieh
force-pushed
the
pieh/worker-pool-types
branch
from
1b51012 to
c0f879c
Compare
pieh
added
topic: TypeScript
| ? MaybeFunction | ||
| : MaybeFunction extends (...args: Array<any>) => any | ||
| ? WrapReturnOfAFunctionInAPromise<MaybeFunction> | ||
| : never |
There was a problem hiding this comment.
never to disallow non-functions if export is not a function
There was a problem hiding this comment.
This is complex XD isn't there another way to accomplish the same or not really?
There was a problem hiding this comment.
My TS-foo is not strong, so not sure about simpler way (at least to have those types automatically) - there might, I just don't know of any
I initially just used exported types from child module as they are (the way jest itself does it - https://github.com/facebook/jest/blob/5f4dd187d89070d07617444186684c20d9213031/packages/jest-runner/src/index.ts#L38-L40), but if you have sync method in child - using it on workerPool would return promise anyway, so types were not always correct.
I guess simple approach would be to define types of "exposed functions" manually, but then it's possibly error-prone and easy to get out of sync.
| export type CreateWorkerPoolType<ExposedFunctions> = Worker & | ||
| { | ||
| [FunctionName in keyof ExposedFunctions]: EnsureFunctionReturnsAPromise< | ||
| ExposedFunctions[FunctionName] | ||
| > | ||
| } |
There was a problem hiding this comment.
WorkerPool type creator, now used just for main workerPool, but in following PRs will be used for tests where test workerPool will include additional helper functions for tests
| @@ -1,3 +1,5 @@ | |||
| /* eslint-disable @typescript-eslint/no-namespace, @typescript-eslint/naming-convention */ | |||
There was a problem hiding this comment.
Our lint rules want to discourage declaring namespace in source code files. Unfortunately having it in declaration file seemed to cause problems for packages other than gatsby ( https://app.circleci.com/pipelines/github/gatsbyjs/gatsby/64194/workflows/e70153e0-70d7-45b6-afa7-a7dcaf4a3471/jobs/707758 was failing for gatsby-plugin-graphql-config).
pieh
force-pushed
the
pieh/worker-pool-types
branch
from
cc31a2d to
304208c
Compare
| @@ -23,7 +23,7 @@ export async function bootstrap( | |||
| initialContext: Partial<IBuildContext> | |||
| ): Promise<{ | |||
| gatsbyNodeGraphQLFunction: Runner | |||
| workerPool: JestWorker | |||
| workerPool: GatsbyWorkerPool | |||
| activity: IActivity, | ||
| htmlComponentRendererPath: string, | ||
| pages: Array<string>, | ||
| stage: Stage = Stage.BuildHTML | ||
| ): Promise<void> => { | ||
| // We need to only pass env vars that are set programmatically in gatsby-cli | ||
| // to child process. Other vars will be picked up from environment. | ||
| const envVars = [ | ||
| const envVars: Array<[string, string | undefined]> = [ |
There was a problem hiding this comment.
can this be undefined, or is this because of process.env having the undefined type?
There was a problem hiding this comment.
It's just process.env.X will be string | undefined - so it technically could undefined if it wasn't setup before. We could force TS into making those just string but then it's not really checking for type safety 🤷
| ? MaybeFunction | ||
| : MaybeFunction extends (...args: Array<any>) => any | ||
| ? WrapReturnOfAFunctionInAPromise<MaybeFunction> | ||
| : never |
There was a problem hiding this comment.
This is complex XD isn't there another way to accomplish the same or not really?
Description
Currently our worker pool has type
anywhen actually interacting with it (inbuild-html.ts), this just adds types to exposed functions