refactor(gatsby): Change develop command to use startWebpackServer service #24946
Conversation
gatsbot
bot
added
the
status: triage needed
ascorbic
added
status: needs core review
|
(I've put it to draft mode since that's what it is) |
|
@pvdz No, it's ready for review. It's just dependent on the other PR |
|
@ascorbic what's up with the added commits? 🤔 |
|
@pvdz Merging master, via the base branch. It initally showed all of those commits before it realised the base branch had been updated too |
Co-authored-by: Peter van der Zee <[email protected]>
… into chore/bootstrap-refactor
| ): Promise<{ gatsbyNodeGraphQLFunction: Runner }> { | ||
| ): Promise<{ | ||
| gatsbyNodeGraphQLFunction: Runner | ||
| workerPool: JestWorker |
There was a problem hiding this comment.
Are we just picking workerPool from context here or the workerPool is created someone in here?
If we are not creating it, it feels weird to return it from bootstrap and not just grab it from context if needed?
There was a problem hiding this comment.
We're getting it at line 35, where we spread in the results of initialize.
|
@ascorbic is it possible to add some comments to say what actually changed, it feels like it's lot of moving pieces around and I want to make sure the changed bits are thoroughly reviewed. Also should we put some tests in place or should we just really on e2e tests? |
The new files should be virtually identical code. It's annoying that there's no easy way to track that. I'm not sure the best way of testing it, because it's mostly about spinning up webpack |
| import { printDeprecationWarnings } from "../utils/print-deprecation-warnings" | ||
| import { printInstructions } from "../utils/print-instructions" | ||
| import { prepareUrls } from "../utils/prepare-urls" | ||
| import { startServer } from "../utils/start-server" |
There was a problem hiding this comment.
bikesheddy: given how all those utils are only used by this service, could we group them in some directory and not just add 4 more files to utils?
so maybe:
- services/
- start-webpack-server/
- index.ts (this file)
- print-deprecation-warnings (utils - repeat for other util)
There was a problem hiding this comment.
I like the cut of your jib
| workerPool: JestWorker | ||
| } | ||
|
|
||
| export async function startServer( |
There was a problem hiding this comment.
Here I've changed the signature to accept the app and workerPool from outside, rather than creating them here
This PR extracts the nitty-gritty of the starting of the webpack dev server and CLI bootstrapping from the
develop-processcommand into astartWebpackServerservice. In the state machine branch this is spawned during bootstrap. It also extracts some of the functions used by that into seperate utils files.