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.

Sign up for GitHub

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

Jump to bottom

fix: check for dirty pages when nodes are deleted (so queries are ru-run and data is removed from pages) #11831

Merged
merged 11 commits into from Feb 20, 2019
Merged

fix: check for dirty pages when nodes are deleted (so queries are ru-run and data is removed from pages) #11831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6d57b16
fix: check for dirty pages when nodes are deleted (so queries are ru-…
KyleAMathews Feb 17, 2019
c3b9ec9
typo fix
KyleAMathews Feb 17, 2019
eb16fd8
Use fake API call as suggested by @pieh
KyleAMathews Feb 17, 2019
a9d9a2e
better name
KyleAMathews Feb 17, 2019
7ffe5c7
Fix comparision as page.updatedAt is a unix timestamp
KyleAMathews Feb 18, 2019
5b5417f
Remove page when deleted from internal query running cache so it can …
KyleAMathews Feb 18, 2019
190625f
WIP
KyleAMathews Feb 18, 2019
890fb36
MORE WIP
KyleAMathews Feb 19, 2019
dcfc73c
Revert "MORE WIP"
KyleAMathews Feb 20, 2019
863acf1
Revert "WIP"
KyleAMathews Feb 20, 2019
516b42c
Document FAKE_API_CALL
KyleAMathews Feb 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/gatsby/src/bootstrap/page-hot-reloader.js
View file
Open in desktop
Expand Up @@ -16,6 +16,11 @@ emitter.on(`CREATE_NODE`, action => {
emitter.on(`DELETE_NODE`, action => {
if (action.payload.internal.type !== `SitePage`) {
pagesDirty = true
// Make a fake API call to trigger `API_RUNNING_QUEUE_EMPTY` being called.
// We don't want to call runCreatePages here as there might be work in
// progress. So this is a safe way to make sure runCreatePages gets called
// at a safe time.
apiRunnerNode(`FAKE_API_CALL`)
}
})

Expand Down Expand Up @@ -45,7 +50,7 @@ const runCreatePages = async () => {
})
.map(p => p.id)

const timestamp = new Date().toJSON()
const timestamp = Date.now()
Copy link
Contributor

Choose a reason for hiding this comment

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

new Date().toJSON() => "2019-02-20T10:47:41.468Z"
Date.now() => 1550659670910

I don't know how page.updatedAt looks so no idea if this is good or bad 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah... for some reason I made a few types (like page) use unix timestamps instead of iso8601 :-(

So this check was failing before.


await apiRunnerNode(`createPages`, {
graphql,
Expand Down
9 changes: 9 additions & 0 deletions packages/gatsby/src/internal-plugins/query-runner/page-query-runner.js
View file
Open in desktop
Expand Up @@ -93,6 +93,15 @@ exports.runQueuedActions = runQueuedActions
emitter.on(`API_RUNNING_QUEUE_EMPTY`, runQueuedActions)

let seenIdsWithoutDataDependencies = []

// Remove pages from seenIdsWithoutDataDependencies when they're deleted
// so their query will be run again if they're created again.
emitter.on(`DELETE_PAGE`, action => {
seenIdsWithoutDataDependencies = seenIdsWithoutDataDependencies.filter(
p => p !== action.payload.path
)
})

const findIdsWithoutDataDependencies = () => {
const state = store.getState()
const allTrackedIds = _.uniq(
Expand Down
6 changes: 5 additions & 1 deletion packages/gatsby/src/utils/api-runner-node.js
View file
Open in desktop
Expand Up @@ -157,7 +157,11 @@ module.exports = async (api, args = {}, pluginSource) =>
})

// Check that the API is documented.
if (!apiList[api]) {
// "FAKE_API_CALL" is used when code needs to trigger something
// to happen once the the API queue is empty. Ideally of course
// we'd have an API (returning a promise) for that. But this
// works nicely in the meantime.
if (!apiList[api] && api !== `FAKE_API_CALL`) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't see FAKE_API_CALL if you could point me in the right direction that would be awesome

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh lemme add a comment on this. It's a bit weird :-D

Copy link
Contributor

Choose a reason for hiding this comment

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

I missed https://github.com/gatsbyjs/gatsby/pull/11831/files#diff-deed9a141e906a882e5f3b2d2c4e21aeR19 my bad :) it makes sense

reporter.panic(`api: "${api}" is not a valid Gatsby api`)
}

Expand Down