From 3d0d3e4f0ae1dafa091cddd4ce05679d7beeaf5b Mon Sep 17 00:00:00 2001 From: Ross Hendry Date: Mon, 29 Jun 2020 17:20:42 +0100 Subject: [PATCH 1/8] WIP --- ...-store.js => better-queue-custom-store.ts} | 14 +-- ...-store.js => better-queue-custom-store.ts} | 88 ++++++++++++------- 2 files changed, 64 insertions(+), 38 deletions(-) rename packages/gatsby/src/query/__tests__/{better-queue-custom-store.js => better-queue-custom-store.ts} (96%) rename packages/gatsby/src/query/{better-queue-custom-store.js => better-queue-custom-store.ts} (61%) diff --git a/packages/gatsby/src/query/__tests__/better-queue-custom-store.js b/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts similarity index 96% rename from packages/gatsby/src/query/__tests__/better-queue-custom-store.js rename to packages/gatsby/src/query/__tests__/better-queue-custom-store.ts index 1f07518559d2d..2f372f69d84ab 100644 --- a/packages/gatsby/src/query/__tests__/better-queue-custom-store.js +++ b/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts @@ -1,5 +1,5 @@ -const MemoryStoreWithPriorityBuckets = require(`../better-queue-custom-store`) -const pify = require(`pify`) +import { MemoryStoreWithPriorityBuckets } from "../better-queue-custom-store" +import pify from "pify" // those are tests copied from https://github.com/diamondio/better-queue-store-test/blob/master/tester.js // and converted from mocha to jest + used pify to make it nicer to read than callback chain @@ -18,6 +18,7 @@ describe(`Custom better-queue memory store`, () => { `releaseLock`, ] beforeEach(() => { + // eslint-disable-next-line new-cap store = MemoryStoreWithPriorityBuckets() functions.forEach(fnName => { if (store[fnName]) { @@ -51,7 +52,8 @@ describe(`Custom better-queue memory store`, () => { await store.putTask(`task2`, { value: `secret 2` }, 1) await store.putTask(`task3`, { value: `secret 3` }, 1) - let lockId, tasks + let lockId: number + let tasks: any lockId = await store.takeLastN(2) tasks = await store.getLock(lockId) @@ -80,7 +82,8 @@ describe(`Custom better-queue memory store`, () => { await store.putTask(`task2`, { value: `secret 2` }, 1) await store.putTask(`task3`, { value: `secret 3` }, 1) - let lockId, tasks + let lockId + let tasks lockId = await store.takeFirstN(2) tasks = await store.getLock(lockId) @@ -173,7 +176,8 @@ describe(`Custom better-queue memory store`, () => { await store.putTask(`task4`, { value: `secret 4` }, 2) // take first 2 - let lockId, tasks + let lockId + let tasks lockId = await store.takeFirstN(2) tasks = await store.getLock(lockId) diff --git a/packages/gatsby/src/query/better-queue-custom-store.js b/packages/gatsby/src/query/better-queue-custom-store.ts similarity index 61% rename from packages/gatsby/src/query/better-queue-custom-store.js rename to packages/gatsby/src/query/better-queue-custom-store.ts index 82afc517a5840..dae857353734f 100644 --- a/packages/gatsby/src/query/better-queue-custom-store.js +++ b/packages/gatsby/src/query/better-queue-custom-store.ts @@ -1,32 +1,53 @@ -function MemoryStoreWithPriorityBuckets() { +type UnknownCallback = (err?: unknown, value?: unknown) => void +type NumberCallback = (err?: unknown, value?: number) => void +type EmptyCallback = () => void + +interface IMemoryStore { + connect(cb: NumberCallback): void + getTask(taskId: string, cb: UnknownCallback): void + deleteTask(taskId: string, cb: EmptyCallback): void + putTask( + taskId: string, + task: unknown, + priority: number, + cb: EmptyCallback + ): void + takeFirstN(n: number, cb: NumberCallback): void + takeLastN(n: number, cb: NumberCallback): void + getRunningTasks(cb: UnknownCallback): void + getLock(lockId: string, cb: UnknownCallback): void + releaseLock(lockId: string, cb: EmptyCallback): void +} + +export function MemoryStoreWithPriorityBuckets(): IMemoryStore { let uuid = 0 /** * Task ids grouped by priority */ - const queueMap = new Map() + const queueMap = new Map() /** * Task id to task lookup */ - const tasks = new Map() + const tasks = new Map() /** * Task id to priority lookup */ - const taskIdToPriority = new Map() + const taskIdToPriority = new Map() /** * Lock to running tasks object */ - const running = {} + const running: Record = {} - let priorityKeys = [] - const updatePriorityKeys = () => { + let priorityKeys: number[] = [] + const updatePriorityKeys = (): void => { priorityKeys = Array.from(queueMap.keys()).sort((a, b) => b - a) } - const addTaskWithPriority = (taskId, priority) => { + const addTaskWithPriority = (taskId: string, priority: number): boolean => { let needToUpdatePriorityKeys = false let priorityTasks = queueMap.get(priority) if (!priorityTasks) { @@ -41,36 +62,38 @@ function MemoryStoreWithPriorityBuckets() { } return { - connect: function (cb) { + connect: function (cb): void { cb(null, tasks.size) }, - getTask: function (taskId, cb) { + getTask: function (taskId, cb): void { cb(null, tasks.get(taskId)) }, - deleteTask: function (taskId, cb) { + deleteTask: function (taskId, cb): void { if (tasks.get(taskId)) { tasks.delete(taskId) const priority = taskIdToPriority.get(taskId) - const priorityTasks = queueMap.get(priority) - priorityTasks.splice(priorityTasks.indexOf(taskId), 1) - taskIdToPriority.delete(taskId) + if (priority) { + const priorityTasks = queueMap.get(priority) || [] + priorityTasks.splice(priorityTasks.indexOf(taskId), 1) + taskIdToPriority.delete(taskId) + } } cb() }, - putTask: function (taskId, task, priority = 0, cb) { + putTask: function (taskId, task, priority = 0, cb): void { const oldTask = tasks.get(taskId) tasks.set(taskId, task) let needToUpdatePriorityKeys = false if (oldTask) { const oldPriority = taskIdToPriority.get(taskId) - if (oldPriority !== priority) { - const oldPriorityTasks = queueMap.get(oldPriority) + if (oldPriority && oldPriority !== priority) { + const oldPriorityTasks = queueMap.get(oldPriority) || [] oldPriorityTasks.splice(oldPriorityTasks.indexOf(taskId), 1) if ( addTaskWithPriority(taskId, priority) || - oldPriority.length === 0 + oldPriorityTasks.length === 0 ) { needToUpdatePriorityKeys = true } @@ -84,7 +107,7 @@ function MemoryStoreWithPriorityBuckets() { } cb() }, - takeFirstN: function (n, cb) { + takeFirstN: function (n, cb): void { const lockId = uuid++ let remainingTasks = n let needToUpdatePriorityKeys = false @@ -92,8 +115,9 @@ function MemoryStoreWithPriorityBuckets() { const tasksToRun = {} for (const priority of priorityKeys) { - const taskWithSamePriority = queueMap.get(priority) - const grabbedTaskIds = taskWithSamePriority.splice(0, remainingTasks) + const tasksWithSamePriority = queueMap.get(priority) + const grabbedTaskIds = + tasksWithSamePriority?.splice(0, remainingTasks) ?? [] grabbedTaskIds.forEach(taskId => { // add task to task that will run // and remove it from waiting list @@ -104,7 +128,7 @@ function MemoryStoreWithPriorityBuckets() { }) remainingTasks -= grabbedTaskIds.length - if (taskWithSamePriority.length === 0) { + if (tasksWithSamePriority?.length === 0) { queueMap.delete(priority) needToUpdatePriorityKeys = true } @@ -123,7 +147,7 @@ function MemoryStoreWithPriorityBuckets() { cb(null, lockId) }, - takeLastN: function (n, cb) { + takeLastN: function (n, cb): void { // This is not really used by Gatsby, but will be implemented for // completion in easiest possible way (so not very performant). // Mostly done so generic test suite used by other stores passes. @@ -136,13 +160,13 @@ function MemoryStoreWithPriorityBuckets() { const tasksToRun = {} for (const priority of priorityKeys.reverse()) { - const taskWithSamePriority = queueMap.get(priority) + const tasksWithSamePriority = queueMap.get(priority) || [] const deleteCount = Math.min( remainingTasks, - taskWithSamePriority.length + tasksWithSamePriority.length ) - const grabbedTaskIds = taskWithSamePriority.splice( - taskWithSamePriority.length - deleteCount, + const grabbedTaskIds = tasksWithSamePriority.splice( + tasksWithSamePriority.length - deleteCount, deleteCount ) grabbedTaskIds.forEach(taskId => { @@ -155,7 +179,7 @@ function MemoryStoreWithPriorityBuckets() { }) remainingTasks -= grabbedTaskIds.length - if (taskWithSamePriority.length === 0) { + if (tasksWithSamePriority.length === 0) { queueMap.delete(priority) needToUpdatePriorityKeys = true } @@ -174,17 +198,15 @@ function MemoryStoreWithPriorityBuckets() { cb(null, lockId) }, - getRunningTasks: function (cb) { + getRunningTasks: function (cb): void { cb(null, running) }, - getLock: function (lockId, cb) { + getLock: function (lockId, cb): void { cb(null, running[lockId]) }, - releaseLock: function (lockId, cb) { + releaseLock: function (lockId, cb): void { delete running[lockId] cb() }, } } - -module.exports = MemoryStoreWithPriorityBuckets From 05809dc5d63fe0a5e8592ebfbfa33f07cb4e82e0 Mon Sep 17 00:00:00 2001 From: Ross Hendry Date: Mon, 29 Jun 2020 18:06:41 +0100 Subject: [PATCH 2/8] chore: Migrate custom store to TypeScript This migrates the custom store and the queue to typescript. --- package.json | 1 + .../src/query/better-queue-custom-store.ts | 53 +++++------ .../gatsby/src/query/{queue.js => queue.ts} | 87 ++++++++++++------- yarn.lock | 9 +- 4 files changed, 85 insertions(+), 65 deletions(-) rename packages/gatsby/src/query/{queue.js => queue.ts} (53%) diff --git a/package.json b/package.json index c4297f4128800..49704d496120c 100644 --- a/package.json +++ b/package.json @@ -6,6 +6,7 @@ "@babel/runtime": "^7.10.3", "@lerna/prompt": "3.18.5", "@types/babel__code-frame": "^7.0.1", + "@types/better-queue": "^3.8.2", "@types/bluebird": "^3.5.30", "@types/cache-manager": "^2.10.2", "@types/common-tags": "^1.8.0", diff --git a/packages/gatsby/src/query/better-queue-custom-store.ts b/packages/gatsby/src/query/better-queue-custom-store.ts index dae857353734f..7429270dd7fc9 100644 --- a/packages/gatsby/src/query/better-queue-custom-store.ts +++ b/packages/gatsby/src/query/better-queue-custom-store.ts @@ -1,25 +1,12 @@ -type UnknownCallback = (err?: unknown, value?: unknown) => void -type NumberCallback = (err?: unknown, value?: number) => void -type EmptyCallback = () => void - -interface IMemoryStore { - connect(cb: NumberCallback): void - getTask(taskId: string, cb: UnknownCallback): void - deleteTask(taskId: string, cb: EmptyCallback): void - putTask( - taskId: string, - task: unknown, - priority: number, - cb: EmptyCallback - ): void - takeFirstN(n: number, cb: NumberCallback): void - takeLastN(n: number, cb: NumberCallback): void - getRunningTasks(cb: UnknownCallback): void - getLock(lockId: string, cb: UnknownCallback): void - releaseLock(lockId: string, cb: EmptyCallback): void +import { Store } from "better-queue" + +// getRunningTasks is an extension to the interface, and is used in the tests +interface IGatsbyBetterStore extends Store { + getRunningTasks(cb: (error: any, runningTasks: any) => void): void } -export function MemoryStoreWithPriorityBuckets(): IMemoryStore { +export function MemoryStoreWithPriorityBuckets(): IGatsbyBetterStore { + type RunningTasks = Record let uuid = 0 /** @@ -30,7 +17,7 @@ export function MemoryStoreWithPriorityBuckets(): IMemoryStore { /** * Task id to task lookup */ - const tasks = new Map() + const tasks = new Map() /** * Task id to priority lookup @@ -40,7 +27,7 @@ export function MemoryStoreWithPriorityBuckets(): IMemoryStore { /** * Lock to running tasks object */ - const running: Record = {} + const running: Record = {} let priorityKeys: number[] = [] const updatePriorityKeys = (): void => { @@ -66,6 +53,7 @@ export function MemoryStoreWithPriorityBuckets(): IMemoryStore { cb(null, tasks.size) }, getTask: function (taskId, cb): void { + // @ts-ignore cb(null, tasks.get(taskId)) }, deleteTask: function (taskId, cb): void { @@ -105,14 +93,14 @@ export function MemoryStoreWithPriorityBuckets(): IMemoryStore { if (needToUpdatePriorityKeys) { updatePriorityKeys() } - cb() + cb(null) }, takeFirstN: function (n, cb): void { - const lockId = uuid++ + const lockId = `` + uuid++ let remainingTasks = n let needToUpdatePriorityKeys = false let haveSomeTasks = false - const tasksToRun = {} + const tasksToRun: RunningTasks = {} for (const priority of priorityKeys) { const tasksWithSamePriority = queueMap.get(priority) @@ -121,10 +109,13 @@ export function MemoryStoreWithPriorityBuckets(): IMemoryStore { grabbedTaskIds.forEach(taskId => { // add task to task that will run // and remove it from waiting list - tasksToRun[taskId] = tasks.get(taskId) - tasks.delete(taskId) - taskIdToPriority.delete(taskId) - haveSomeTasks = true + const task = tasks.get(taskId) + if (task) { + tasksToRun[taskId] = task + tasks.delete(taskId) + taskIdToPriority.delete(taskId) + haveSomeTasks = true + } }) remainingTasks -= grabbedTaskIds.length @@ -153,7 +144,7 @@ export function MemoryStoreWithPriorityBuckets(): IMemoryStore { // Mostly done so generic test suite used by other stores passes. // This is mostly C&P from takeFirstN, with array reversal and different // splice args - const lockId = uuid++ + const lockId = `` + uuid++ let remainingTasks = n let needToUpdatePriorityKeys = false let haveSomeTasks = false @@ -206,7 +197,7 @@ export function MemoryStoreWithPriorityBuckets(): IMemoryStore { }, releaseLock: function (lockId, cb): void { delete running[lockId] - cb() + cb(null) }, } } diff --git a/packages/gatsby/src/query/queue.js b/packages/gatsby/src/query/queue.ts similarity index 53% rename from packages/gatsby/src/query/queue.js rename to packages/gatsby/src/query/queue.ts index ed80fb7d6cbf6..b05e7318f6876 100644 --- a/packages/gatsby/src/query/queue.js +++ b/packages/gatsby/src/query/queue.ts @@ -1,23 +1,34 @@ -const Queue = require(`better-queue`) -const { store } = require(`../redux`) -const FastMemoryStore = require(`../query/better-queue-custom-store`) -const { queryRunner } = require(`../query/query-runner`) -const { websocketManager } = require(`../utils/websocket-manager`) -const { GraphQLRunner } = require(`./graphql-runner`) - -const createBaseOptions = () => { +import Queue from "better-queue" +import { store } from "../redux" +import { MemoryStoreWithPriorityBuckets } from "../query/better-queue-custom-store" +import { queryRunner } from "../query/query-runner" +import { websocketManager } from "../utils/websocket-manager" +import { GraphQLRunner } from "./graphql-runner" +import BetterQueue from "better-queue" +import { IExecutionResult } from "./types" +import { ProgressActivityTracker } from "../.." + +export type Task = any +type TaskResult = any + +const createBaseOptions = (): Partial< + BetterQueue.QueueOptions +> => { return { concurrent: Number(process.env.GATSBY_EXPERIMENTAL_QUERY_CONCURRENCY) || 4, // eslint-disable-next-line new-cap - store: FastMemoryStore(), + store: MemoryStoreWithPriorityBuckets(), } } -const createBuildQueue = (graphqlRunner, runnerOptions = {}) => { +const createBuildQueue = ( + graphqlRunner: GraphQLRunner, + runnerOptions = {} +): Queue => { if (!graphqlRunner) { graphqlRunner = new GraphQLRunner(store, runnerOptions) } - const handler = ({ job, activity }, callback) => + const handler = ({ job, activity }, callback): Promise => queryRunner(graphqlRunner, job, activity?.span) .then(result => callback(null, result)) .catch(callback) @@ -25,22 +36,26 @@ const createBuildQueue = (graphqlRunner, runnerOptions = {}) => { return queue } -const createDevelopQueue = getRunner => { +const createDevelopQueue = (getRunner: () => GraphQLRunner): Queue => { const queueOptions = { ...createBaseOptions(), - priority: ({ job }, cb) => { + priority: ({ job }, cb): void => { if (job.id && websocketManager.activePaths.has(job.id)) { cb(null, 10) } else { cb(null, 1) } }, - merge: (oldTask, newTask, cb) => { + merge: ( + oldTask: Task, + newTask: Task, + cb: (err?: unknown, newTask?: Task) => void + ): void => { cb(null, newTask) }, } - const handler = ({ job: queryJob, activity }, callback) => { + const handler = ({ job: queryJob, activity }, callback): void => { queryRunner(getRunner(), queryJob, activity?.span).then( result => { if (!queryJob.isPage) { @@ -65,37 +80,47 @@ const createDevelopQueue = getRunner => { * fail) * Note: queue is reused in develop so make sure to thoroughly cleanup hooks */ -const processBatch = async (queue, jobs, activity) => { +const processBatch = async ( + queue: Queue, + jobs: Task[], + activity: ProgressActivityTracker +): Promise => { if (jobs.length === 0) { return Promise.resolve() } return new Promise((resolve, reject) => { let taskFinishCallback + + const gc = (): void => { + // eslint-disable-next-line @typescript-eslint/no-use-before-define + queue.off(`task_failed`, taskFailedCallback) + // eslint-disable-next-line @typescript-eslint/no-use-before-define + queue.off(`drain`, drainCallback) + if (taskFinishCallback) { + queue.off(`task_finish`, taskFinishCallback) + } + // We don't want to allow the variable to be null any other time, + // just when marking it as eligible for garbage collection. + // @ts-ignore + queue = null + } + if (activity.tick) { - taskFinishCallback = () => activity.tick() + taskFinishCallback = (): unknown => activity.tick() queue.on(`task_finish`, taskFinishCallback) } - const taskFailedCallback = (...err) => { + const taskFailedCallback = (...err: unknown[]): void => { gc() reject(err) } - const drainCallback = () => { + const drainCallback = (): void => { gc() resolve() } - const gc = () => { - queue.off(`task_failed`, taskFailedCallback) - queue.off(`drain`, drainCallback) - if (taskFinishCallback) { - queue.off(`task_finish`, taskFinishCallback) - } - queue = null - } - queue // Note: the first arg is the path, the second the error .on(`task_failed`, taskFailedCallback) @@ -112,8 +137,4 @@ const processBatch = async (queue, jobs, activity) => { }) } -module.exports = { - createBuildQueue, - createDevelopQueue, - processBatch, -} +export { createBuildQueue, createDevelopQueue, processBatch } diff --git a/yarn.lock b/yarn.lock index 1b06b648d2ec4..290e91db685c0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5391,6 +5391,13 @@ dependencies: "@babel/types" "^7.3.0" +"@types/better-queue@^3.8.2": + version "3.8.2" + resolved "https://registry.yarnpkg.com/@types/better-queue/-/better-queue-3.8.2.tgz#911a86863c1dd89a42308e03ee8d25ab7f6bafa7" + integrity sha512-KMFFgojmy10+npJiw/XkY2i+UD96NZmo4L0xuw8DRfEXB4a70rS6YHzqpb7Xgh1+TwWFsIHbsK8fFkvMTxEASA== + dependencies: + "@types/node" "*" + "@types/bluebird@^3.5.30": version "3.5.30" resolved "https://registry.yarnpkg.com/@types/bluebird/-/bluebird-3.5.30.tgz#ee034a0eeea8b84ed868b1aa60d690b08a6cfbc5" @@ -5922,7 +5929,7 @@ "@types/source-list-map" "*" source-map "^0.6.1" -"@types/webpack@*", "@types/webpack@4.41.18", "@types/webpack@^4.41.18": +"@types/webpack@*", "@types/webpack@^4.41.18": version "4.41.18" resolved "https://registry.yarnpkg.com/@types/webpack/-/webpack-4.41.18.tgz#2945202617866ecdffa582087f1b6de04a7eed55" integrity sha512-mQm2R8vV2BZE/qIDVYqmBVLfX73a8muwjs74SpjEyJWJxeXBbsI9L65Pcia9XfYLYWzD1c1V8m+L0p30y2N7MA== From 2d53ce8078627785cfe571f82fc5e4f0557547ff Mon Sep 17 00:00:00 2001 From: Ross Hendry Date: Mon, 29 Jun 2020 18:13:21 +0100 Subject: [PATCH 3/8] Test types --- .../__tests__/better-queue-custom-store.ts | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts b/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts index 2f372f69d84ab..f5ba729bbe025 100644 --- a/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts +++ b/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts @@ -52,11 +52,8 @@ describe(`Custom better-queue memory store`, () => { await store.putTask(`task2`, { value: `secret 2` }, 1) await store.putTask(`task3`, { value: `secret 3` }, 1) - let lockId: number - let tasks: any - - lockId = await store.takeLastN(2) - tasks = await store.getLock(lockId) + let lockId: string = await store.takeLastN(2) + let tasks: any = await store.getLock(lockId) // should get the third task expect(tasks.task3.value).toBe(`secret 3`) @@ -112,8 +109,8 @@ describe(`Custom better-queue memory store`, () => { await store.putTask(`task2`, { value: `secret 2` }, 1) await store.putTask(`task3`, { value: `secret 3` }, 1) - const lock1 = await store.takeFirstN(1) - const lock2 = await store.takeLastN(1) + const lock1: string = await store.takeFirstN(1) + const lock2: string = await store.takeLastN(1) let workers @@ -154,7 +151,7 @@ describe(`Custom better-queue memory store`, () => { await store.deleteTask(`task2`) // take 2 - const lockId = await store.takeFirstN(2) + const lockId: string = await store.takeFirstN(2) const tasks = await store.getLock(lockId) // should get the first task @@ -176,10 +173,8 @@ describe(`Custom better-queue memory store`, () => { await store.putTask(`task4`, { value: `secret 4` }, 2) // take first 2 - let lockId - let tasks - lockId = await store.takeFirstN(2) - tasks = await store.getLock(lockId) + let lockId: string = await store.takeFirstN(2) + let tasks = await store.getLock(lockId) // should get the third task expect(tasks.task3.value).toBe(`secret 3`) From a943c629e82721a107aa0429331d2e5fbf8688ce Mon Sep 17 00:00:00 2001 From: Ross Hendry Date: Mon, 29 Jun 2020 19:32:39 +0100 Subject: [PATCH 4/8] Remove warning --- packages/gatsby/src/query/queue.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby/src/query/queue.ts b/packages/gatsby/src/query/queue.ts index b05e7318f6876..996d48156c003 100644 --- a/packages/gatsby/src/query/queue.ts +++ b/packages/gatsby/src/query/queue.ts @@ -47,7 +47,7 @@ const createDevelopQueue = (getRunner: () => GraphQLRunner): Queue => { } }, merge: ( - oldTask: Task, + _oldTask: Task, newTask: Task, cb: (err?: unknown, newTask?: Task) => void ): void => { From 94025b2ea9956ef059fd11f2db58aad9da9a16f2 Mon Sep 17 00:00:00 2001 From: Ross Hendry Date: Tue, 30 Jun 2020 14:16:14 +0100 Subject: [PATCH 5/8] chore: Pass full config object --- .../__tests__/better-queue-custom-store.ts | 5 +- .../src/query/better-queue-custom-store.ts | 16 ++--- packages/gatsby/src/query/queue.ts | 64 ++++++++++--------- 3 files changed, 43 insertions(+), 42 deletions(-) diff --git a/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts b/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts index f5ba729bbe025..80b8242a70a43 100644 --- a/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts +++ b/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts @@ -1,4 +1,4 @@ -import { MemoryStoreWithPriorityBuckets } from "../better-queue-custom-store" +import { memoryStoreWithPriorityBuckets } from "../better-queue-custom-store" import pify from "pify" // those are tests copied from https://github.com/diamondio/better-queue-store-test/blob/master/tester.js @@ -18,8 +18,7 @@ describe(`Custom better-queue memory store`, () => { `releaseLock`, ] beforeEach(() => { - // eslint-disable-next-line new-cap - store = MemoryStoreWithPriorityBuckets() + store = memoryStoreWithPriorityBuckets() functions.forEach(fnName => { if (store[fnName]) { store[fnName] = pify(store[fnName]) diff --git a/packages/gatsby/src/query/better-queue-custom-store.ts b/packages/gatsby/src/query/better-queue-custom-store.ts index 7429270dd7fc9..2f145534c21ca 100644 --- a/packages/gatsby/src/query/better-queue-custom-store.ts +++ b/packages/gatsby/src/query/better-queue-custom-store.ts @@ -5,14 +5,14 @@ interface IGatsbyBetterStore extends Store { getRunningTasks(cb: (error: any, runningTasks: any) => void): void } -export function MemoryStoreWithPriorityBuckets(): IGatsbyBetterStore { +export function memoryStoreWithPriorityBuckets(): IGatsbyBetterStore { type RunningTasks = Record let uuid = 0 /** * Task ids grouped by priority */ - const queueMap = new Map() + const queueMap = new Map>() /** * Task id to task lookup @@ -29,7 +29,7 @@ export function MemoryStoreWithPriorityBuckets(): IGatsbyBetterStore { */ const running: Record = {} - let priorityKeys: number[] = [] + let priorityKeys: Array = [] const updatePriorityKeys = (): void => { priorityKeys = Array.from(queueMap.keys()).sort((a, b) => b - a) } @@ -61,7 +61,7 @@ export function MemoryStoreWithPriorityBuckets(): IGatsbyBetterStore { tasks.delete(taskId) const priority = taskIdToPriority.get(taskId) if (priority) { - const priorityTasks = queueMap.get(priority) || [] + const priorityTasks = queueMap.get(priority) ?? [] priorityTasks.splice(priorityTasks.indexOf(taskId), 1) taskIdToPriority.delete(taskId) } @@ -76,7 +76,7 @@ export function MemoryStoreWithPriorityBuckets(): IGatsbyBetterStore { const oldPriority = taskIdToPriority.get(taskId) if (oldPriority && oldPriority !== priority) { - const oldPriorityTasks = queueMap.get(oldPriority) || [] + const oldPriorityTasks = queueMap.get(oldPriority) ?? [] oldPriorityTasks.splice(oldPriorityTasks.indexOf(taskId), 1) if ( @@ -96,7 +96,7 @@ export function MemoryStoreWithPriorityBuckets(): IGatsbyBetterStore { cb(null) }, takeFirstN: function (n, cb): void { - const lockId = `` + uuid++ + const lockId = String(uuid++) let remainingTasks = n let needToUpdatePriorityKeys = false let haveSomeTasks = false @@ -144,14 +144,14 @@ export function MemoryStoreWithPriorityBuckets(): IGatsbyBetterStore { // Mostly done so generic test suite used by other stores passes. // This is mostly C&P from takeFirstN, with array reversal and different // splice args - const lockId = `` + uuid++ + const lockId = String(uuid++) let remainingTasks = n let needToUpdatePriorityKeys = false let haveSomeTasks = false const tasksToRun = {} for (const priority of priorityKeys.reverse()) { - const tasksWithSamePriority = queueMap.get(priority) || [] + const tasksWithSamePriority = queueMap.get(priority) ?? [] const deleteCount = Math.min( remainingTasks, tasksWithSamePriority.length diff --git a/packages/gatsby/src/query/queue.ts b/packages/gatsby/src/query/queue.ts index 996d48156c003..d26a38a546d23 100644 --- a/packages/gatsby/src/query/queue.ts +++ b/packages/gatsby/src/query/queue.ts @@ -1,23 +1,22 @@ import Queue from "better-queue" import { store } from "../redux" -import { MemoryStoreWithPriorityBuckets } from "../query/better-queue-custom-store" +import { memoryStoreWithPriorityBuckets } from "../query/better-queue-custom-store" import { queryRunner } from "../query/query-runner" import { websocketManager } from "../utils/websocket-manager" import { GraphQLRunner } from "./graphql-runner" import BetterQueue from "better-queue" -import { IExecutionResult } from "./types" import { ProgressActivityTracker } from "../.." export type Task = any type TaskResult = any -const createBaseOptions = (): Partial< - BetterQueue.QueueOptions +const createBaseOptions = (): Pick< + BetterQueue.QueueOptions, + "concurrent" | "store" > => { return { concurrent: Number(process.env.GATSBY_EXPERIMENTAL_QUERY_CONCURRENCY) || 4, - // eslint-disable-next-line new-cap - store: MemoryStoreWithPriorityBuckets(), + store: memoryStoreWithPriorityBuckets(), } } @@ -28,16 +27,20 @@ const createBuildQueue = ( if (!graphqlRunner) { graphqlRunner = new GraphQLRunner(store, runnerOptions) } - const handler = ({ job, activity }, callback): Promise => - queryRunner(graphqlRunner, job, activity?.span) - .then(result => callback(null, result)) - .catch(callback) - const queue = new Queue(handler, createBaseOptions()) - return queue + + const queueOptions: BetterQueue.QueueOptions = { + ...createBaseOptions(), + process: ({ job, activity }, callback): void => { + queryRunner(graphqlRunner, job, activity?.span) + .then(result => callback(null, result)) + .catch(callback) + }, + } + return new Queue(queueOptions) } const createDevelopQueue = (getRunner: () => GraphQLRunner): Queue => { - const queueOptions = { + const queueOptions: BetterQueue.QueueOptions = { ...createBaseOptions(), priority: ({ job }, cb): void => { if (job.id && websocketManager.activePaths.has(job.id)) { @@ -53,25 +56,24 @@ const createDevelopQueue = (getRunner: () => GraphQLRunner): Queue => { ): void => { cb(null, newTask) }, + process: ({ job: queryJob, activity }, callback): void => { + queryRunner(getRunner(), queryJob, activity?.span).then( + result => { + if (!queryJob.isPage) { + websocketManager.emitStaticQueryData({ + result, + id: queryJob.id, + }) + } + + callback(null, result) + }, + error => callback(error) + ) + }, } - const handler = ({ job: queryJob, activity }, callback): void => { - queryRunner(getRunner(), queryJob, activity?.span).then( - result => { - if (!queryJob.isPage) { - websocketManager.emitStaticQueryData({ - result, - id: queryJob.id, - }) - } - - callback(null, result) - }, - error => callback(error) - ) - } - - return new Queue(handler, queueOptions) + return new Queue(queueOptions) } /** @@ -111,7 +113,7 @@ const processBatch = async ( queue.on(`task_finish`, taskFinishCallback) } - const taskFailedCallback = (...err: unknown[]): void => { + const taskFailedCallback = (...err: Array): void => { gc() reject(err) } From 4551a5230f7e9e9479333ee6fef64d5a29110435 Mon Sep 17 00:00:00 2001 From: Ross Hendry Date: Wed, 1 Jul 2020 08:27:16 +0100 Subject: [PATCH 6/8] Do not leak interface extension details Rather than leaking the extra function to the outside world, we'll keep it internal. The tests do not cause an issue as they're written in a way that erases types. --- .../src/query/better-queue-custom-store.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/gatsby/src/query/better-queue-custom-store.ts b/packages/gatsby/src/query/better-queue-custom-store.ts index 2f145534c21ca..4b90626f65061 100644 --- a/packages/gatsby/src/query/better-queue-custom-store.ts +++ b/packages/gatsby/src/query/better-queue-custom-store.ts @@ -1,11 +1,6 @@ import { Store } from "better-queue" -// getRunningTasks is an extension to the interface, and is used in the tests -interface IGatsbyBetterStore extends Store { - getRunningTasks(cb: (error: any, runningTasks: any) => void): void -} - -export function memoryStoreWithPriorityBuckets(): IGatsbyBetterStore { +export function memoryStoreWithPriorityBuckets(): Store { type RunningTasks = Record let uuid = 0 @@ -80,8 +75,8 @@ export function memoryStoreWithPriorityBuckets(): IGatsbyBetterStore { oldPriorityTasks.splice(oldPriorityTasks.indexOf(taskId), 1) if ( - addTaskWithPriority(taskId, priority) || - oldPriorityTasks.length === 0 + addTaskWithPriority(taskId, priority) // || + // oldPriorityTasks.length === 0 ) { needToUpdatePriorityKeys = true } @@ -189,7 +184,11 @@ export function memoryStoreWithPriorityBuckets(): IGatsbyBetterStore { cb(null, lockId) }, - getRunningTasks: function (cb): void { + // @ts-ignore + // getRunningTasks is an extension to the interface, and is only used in the tests + getRunningTasks: function ( + cb: (err?: any, value?: Record) => void + ): void { cb(null, running) }, getLock: function (lockId, cb): void { From 56bd7e0aaee92f46b5b35072195dacfa2a94f366 Mon Sep 17 00:00:00 2001 From: Ross Hendry Date: Mon, 13 Jul 2020 11:11:31 +0100 Subject: [PATCH 7/8] Bring in queue changes --- packages/gatsby/src/query/graphql-runner.ts | 13 ++++++----- packages/gatsby/src/query/queue.ts | 24 ++++++++++++++++++--- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/packages/gatsby/src/query/graphql-runner.ts b/packages/gatsby/src/query/graphql-runner.ts index d8ff962889fc2..fe5dd94edc3f1 100644 --- a/packages/gatsby/src/query/graphql-runner.ts +++ b/packages/gatsby/src/query/graphql-runner.ts @@ -22,6 +22,11 @@ import GraphQLSpanTracer from "./graphql-span-tracer" type Query = string | Source +export interface IGraphQLRunnerOptions { + collectStats?: boolean + graphqlTracing?: boolean +} + export class GraphQLRunner { parseCache: Map @@ -38,13 +43,7 @@ export class GraphQLRunner { constructor( protected store: Store, - { - collectStats, - graphqlTracing, - }: { - collectStats?: boolean - graphqlTracing?: boolean - } = {} + { collectStats, graphqlTracing }: IGraphQLRunnerOptions = {} ) { const { schema, schemaCustomization } = this.store.getState() diff --git a/packages/gatsby/src/query/queue.ts b/packages/gatsby/src/query/queue.ts index d26a38a546d23..fde96a7bdfbcb 100644 --- a/packages/gatsby/src/query/queue.ts +++ b/packages/gatsby/src/query/queue.ts @@ -3,7 +3,7 @@ import { store } from "../redux" import { memoryStoreWithPriorityBuckets } from "../query/better-queue-custom-store" import { queryRunner } from "../query/query-runner" import { websocketManager } from "../utils/websocket-manager" -import { GraphQLRunner } from "./graphql-runner" +import { GraphQLRunner, IGraphQLRunnerOptions } from "./graphql-runner" import BetterQueue from "better-queue" import { ProgressActivityTracker } from "../.." @@ -22,7 +22,7 @@ const createBaseOptions = (): Pick< const createBuildQueue = ( graphqlRunner: GraphQLRunner, - runnerOptions = {} + runnerOptions: IGraphQLRunnerOptions = {} ): Queue => { if (!graphqlRunner) { graphqlRunner = new GraphQLRunner(store, runnerOptions) @@ -76,6 +76,19 @@ const createDevelopQueue = (getRunner: () => GraphQLRunner): Queue => { return new Queue(queueOptions) } +const createAppropriateQueue = ( + graphqlRunner: GraphQLRunner, + runnerOptions: IGraphQLRunnerOptions = {} +): Queue => { + if (process.env.NODE_ENV === `production`) { + return createBuildQueue(graphqlRunner, runnerOptions) + } + if (!graphqlRunner) { + graphqlRunner = new GraphQLRunner(store, runnerOptions) + } + return createDevelopQueue(() => graphqlRunner) +} + /** * Returns a promise that pushes jobs onto queue and resolves onces * they're all finished processing (or rejects if one or more jobs @@ -139,4 +152,9 @@ const processBatch = async ( }) } -export { createBuildQueue, createDevelopQueue, processBatch } +export { + createBuildQueue, + createDevelopQueue, + processBatch, + createAppropriateQueue, +} From 7b8564dfeeceac28f5354eaaa3955a7d90556f59 Mon Sep 17 00:00:00 2001 From: Ross Hendry Date: Mon, 13 Jul 2020 14:41:10 +0100 Subject: [PATCH 8/8] Initialize and declare initial values --- .../src/query/__tests__/better-queue-custom-store.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts b/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts index 80b8242a70a43..7b3171001ed33 100644 --- a/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts +++ b/packages/gatsby/src/query/__tests__/better-queue-custom-store.ts @@ -78,11 +78,8 @@ describe(`Custom better-queue memory store`, () => { await store.putTask(`task2`, { value: `secret 2` }, 1) await store.putTask(`task3`, { value: `secret 3` }, 1) - let lockId - let tasks - - lockId = await store.takeFirstN(2) - tasks = await store.getLock(lockId) + let lockId = await store.takeFirstN(2) + let tasks = await store.getLock(lockId) // should get the first task expect(tasks.task1.value).toBe(`secret 1`) @@ -111,9 +108,7 @@ describe(`Custom better-queue memory store`, () => { const lock1: string = await store.takeFirstN(1) const lock2: string = await store.takeLastN(1) - let workers - - workers = await store.getRunningTasks() + let workers = await store.getRunningTasks() // should have first lock expect(workers[lock1]).toBeDefined()