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

fix(gatsby): PnP fixes #35194

Merged
merged 6 commits into from
Mar 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,6 @@ jobs:
- run: # Quick upgrade to the v2 (any version, we just need the real set version)
command: yarn policies set-version berry
working_directory: /tmp/e2e-tests/gatsby-pnp
- run: # TODO: remove pinned version
command: yarn set version 3.1.1
working_directory: /tmp/e2e-tests/gatsby-pnp
Comment on lines -353 to -355
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unpin yarn version in tests, so we start using latest again

- run: # Explicitly set nodeLinker to avoid Yarn selecting node_modules due to the Yarn 1.x lockfile
command: yarn config set nodeLinker pnp
working_directory: /tmp/e2e-tests/gatsby-pnp
Expand Down
3 changes: 3 additions & 0 deletions packages/gatsby-worker/src/child.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
ERROR,
RESULT,
CUSTOM_MESSAGE,
WORKER_READY,
} from "./types"
import { isPromise } from "./utils"

Expand Down Expand Up @@ -102,6 +103,8 @@ if (process.send && process.env.GATSBY_WORKER_MODULE_PATH) {
}

process.on(`message`, messageHandler)

ensuredSendToMain([WORKER_READY])
Comment on lines +106 to +107
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We didn't have init messaging and seems like in some cases initial messages (before child set on('message') handler) are lost. The change here is that child sends "ready" message once it's initialized and workerpool waits for it before sending task to process

}

export { isWorker, getMessenger }
14 changes: 12 additions & 2 deletions packages/gatsby-worker/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
ERROR,
RESULT,
CUSTOM_MESSAGE,
WORKER_READY,
ParentMessageUnion,
ChildMessageUnion,
} from "./types"
Expand Down Expand Up @@ -87,6 +88,7 @@ interface IWorkerInfo<T> {
signal: NodeJS.Signals | null
}>
currentTask?: TaskInfo<T>
ready: Promise<void>
}

export interface IPublicWorkerInfo {
Expand Down Expand Up @@ -183,9 +185,13 @@ export class WorkerPool<
silent: options && options.silent,
})

let workerReadyResolve: () => void
const workerInfo: IWorkerInfo<keyof WorkerModuleExports> = {
workerId,
worker,
ready: new Promise<void>(resolve => {
workerReadyResolve = resolve
}),
exitedPromise: new Promise(resolve => {
worker.on(`exit`, (code, signal) => {
if (workerInfo.currentTask) {
Expand Down Expand Up @@ -247,6 +253,8 @@ export class WorkerPool<
for (const listener of this.listeners) {
listener(msg[1] as MessagesFromChild, workerId)
}
} else if (msg[0] === WORKER_READY) {
workerReadyResolve()
}
})

Expand Down Expand Up @@ -322,14 +330,16 @@ export class WorkerPool<
this.idleWorkers.add(workerInfo)
}

private doWork<T extends keyof WorkerModuleExports>(
private async doWork<T extends keyof WorkerModuleExports>(
taskInfo: TaskInfo<T>,
workerInfo: IWorkerInfo<T>
): void {
): Promise<void> {
// block worker
workerInfo.currentTask = taskInfo
this.idleWorkers.delete(workerInfo)

await workerInfo.ready

const msg: ParentMessageUnion = [
EXECUTE,
taskInfo.functionName,
Expand Down
8 changes: 7 additions & 1 deletion packages/gatsby-worker/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export const ERROR = 0b10
export const RESULT = 0b11
export const END = 0b00
export const CUSTOM_MESSAGE = 0b100
export const WORKER_READY = 0b1000

type CustomMessage = [typeof CUSTOM_MESSAGE, unknown]

Expand All @@ -11,6 +12,7 @@ type FunctionArgs = Array<any>

type ExecuteMessage = [typeof EXECUTE, FunctionName, FunctionArgs]
type EndMessage = [typeof END]
type WorkerReadyMessage = [typeof WORKER_READY]

export type ParentMessageUnion = ExecuteMessage | EndMessage | CustomMessage

Expand All @@ -30,4 +32,8 @@ type ResultType = unknown

type TaskResult = [typeof RESULT, ResultType]

export type ChildMessageUnion = TaskError | TaskResult | CustomMessage
export type ChildMessageUnion =
| TaskError
| TaskResult
| CustomMessage
| WorkerReadyMessage
4 changes: 1 addition & 3 deletions packages/gatsby/src/utils/parcel/compile-gatsby-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ export function constructParcel(siteRoot: string): Parcel {
`${siteRoot}/${gatsbyFileRegex}`,
`${siteRoot}/plugins/**/${gatsbyFileRegex}`,
],
defaultConfig: require.resolve(`gatsby-parcel-config`, {
paths: [siteRoot],
}),
defaultConfig: require.resolve(`gatsby-parcel-config`),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

gatsby-parcel-config is dependency of gatsby not site, so we shouldn't try to resolve it from site context

mode: `production`,
targets: {
root: {
Expand Down