feat(gatsby): Instrument partial writes to page data #24808
Conversation
sidharthachatterjee
added
the
type: feature or enhancement
gatsbot
bot
added
the
status: triage needed
sidharthachatterjee
removed
the
status: triage needed
|
Waiting on a blessing from Master Jedi @pieh |
sidharthachatterjee
added
the
bot: merge on green
| store.dispatch({ | ||
| type: `CLEAR_PENDING_PAGE_DATA_WRITES`, | ||
| }) |
There was a problem hiding this comment.
@sidharthachatterjee we introduced a bug with this (thank @TylerBarnes for raising this issue)
we generate pagesToWrite at the beginning of function (sync), and then the loop is async, which means that some concurrent code can add pending pages/templates while the loop is running and after that we clear all pending data writes which means that pending writes added mid function run are never handled.
What we did with Tyler to debug here is we rerun code used to generate pagesToWrite after the loop and realized that that it doesn't match with one we generated at the beginning of the function.
I think we either need to make this entire flush sync (yikes) or add some listener (e.g. emitter.on("ADD_PENDING_PAGE_DATA_WRITE", ...) that would flip some switch and rerun flush or something before dispatching CLEAR_PENDING_PAGE_DATA_WRITES
There was a problem hiding this comment.
For context this bug makes data updates break for WP due to missing page data but only on some sites. It's not reproducible on the default starter and seemingly needs a more complex site to reproduce
There was a problem hiding this comment.
Not sure if that would work - but maybe we should dispatch "clear pending" action immediately after constructing pagesToWrite (as code is sync up to this point), so after the loop we could just check if there is anything pending (meaning it was added since the clear) and then re-run pending? This way we won't need to listen to ADD_PENDING_* actions or make things sync?
There was a problem hiding this comment.
maybe a good use-case for a generator? after each iteration check to see if anything has been added and put it on the end of the processing queue
* Add a pending page data reducer * Add mechanism to flush page data after collecting pending writes * Linting * Fix * Update snapshot * move to program._ instead of command * do not emit page data in query queue * fix check for develop * add a null check * Prevent flush races * Only flush if in queue * Enqueue instead of flushing * Linting * Make enqueue not await * Reset enqueue * Update packages/gatsby/src/commands/build.ts Co-authored-by: Michal Piechowiak <[email protected]> * Linting * immediately flush on build * Add a getter * Move page data back to TS * Use named imports for page data * Use better names * fix typings and remove async Co-authored-by: Michal Piechowiak <[email protected]>
Currently, writing
page-data.jsonfor a page is atomic. But this doesn't work for cases where we need to collect other information (upcoming module dependencies from @pieh and static queries from me)This instruments a mechanism to add a pending update and then flush all pending updates to a page's
page-data.jsonwhen ready.This also adds some naive coordination for webpack so we can time flushing correctly.