Skip to content

Commit

Permalink
fix(gatsby): garbage collect stateful pages (#28760)
Browse files Browse the repository at this point in the history
* add stateful page to artifacts tests

* fix(gatsby): garbage collect stateful pages

* Revert "fix: clear tracked queries when deleting stale page-data files (#29431)" (#30848)

This reverts commit 478cf68.
  • Loading branch information
pieh committed Apr 21, 2021
1 parent 135ddd6 commit 7880240
Show file tree
Hide file tree
Showing 15 changed files with 285 additions and 88 deletions.
3 changes: 3 additions & 0 deletions integration-tests/artifacts/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,7 @@ describe(`Second run (different pages created, data changed)`, () => {
`/static-query-result-tracking/stable/`,
`/static-query-result-tracking/rerun-query-but-dont-recreate-html/`,
`/page-that-will-have-trailing-slash-removed`,
`/stateful-page-not-recreated-in-third-run/`,
]

const expectedPages = [
Expand Down Expand Up @@ -614,6 +615,7 @@ describe(`Third run (js change, all pages are recreated)`, () => {
`/stale-pages/only-in-first/`,
`/page-query-dynamic-1/`,
`/page-query-dynamic-2/`,
`/stateful-page-not-recreated-in-third-run/`,
]

let changedFileOriginalContent
Expand Down Expand Up @@ -699,6 +701,7 @@ describe(`Fourth run (gatsby-browser change - cache get invalidated)`, () => {
const expectedPages = [
`/stale-pages/only-not-in-first`,
`/page-query-dynamic-4/`,
`/stateful-page-not-recreated-in-third-run/`,
]

const unexpectedPages = [
Expand Down
12 changes: 12 additions & 0 deletions integration-tests/artifacts/gatsby-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,18 @@ exports.createPages = async ({ actions, graphql }) => {
}
}

exports.createPagesStatefully = async ({ actions }) => {
if (runNumber !== 3) {
actions.createPage({
path: `/stateful-page-not-recreated-in-third-run/`,
component: require.resolve(`./src/templates/dummy`),
context: {
dummyId: `stateful-page`,
},
})
}
}

exports.onPreBuild = () => {
console.log(`[test] onPreBuild`)
changedBrowserCompilationHash = `not-changed`
Expand Down
8 changes: 4 additions & 4 deletions packages/gatsby/src/bootstrap/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
sourceNodes,
buildSchema,
createPages,
createPagesStatefully,
extractQueries,
writeOutRedirects,
postBootstrap,
Expand All @@ -32,9 +31,12 @@ export async function bootstrap(

const parentSpan = tracer.startSpan(`bootstrap`, spanArgs)

const bootstrapContext: IBuildContext = {
const bootstrapContext: IBuildContext & {
shouldRunCreatePagesStatefully: boolean
} = {
...initialContext,
parentSpan,
shouldRunCreatePagesStatefully: true,
}

const context = {
Expand All @@ -54,8 +56,6 @@ export async function bootstrap(

await createPages(context)

await createPagesStatefully(context)

await handleStalePageData()

await rebuildSchemaWithSitePage(context)
Expand Down
14 changes: 0 additions & 14 deletions packages/gatsby/src/redux/reducers/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,6 @@ export function queriesReducer(
state.deletedQueries.add(action.payload.path)
return state
}
case `DELETED_STALE_PAGE_DATA_FILES`: {
// this action is a hack/hot fix
// it should be removed/reverted when we start persisting pages state
for (const queryId of action.payload.pagePathsToClear) {
for (const component of state.trackedComponents.values()) {
component.pages.delete(queryId)
}
state = clearNodeDependencies(state, queryId)
state = clearConnectionDependencies(state, queryId)
state.trackedQueries.delete(queryId)
}

return state
}
case `API_FINISHED`: {
if (action.payload.apiName !== `createPages`) {
return state
Expand Down
8 changes: 0 additions & 8 deletions packages/gatsby/src/redux/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@ export type ActionsUnion =
| IDisableTypeInferenceAction
| ISetProgramAction
| ISetProgramExtensions
| IDeletedStalePageDataFiles
| IRemovedHtml
| ITrackedHtmlCleanup
| IGeneratedHtml
Expand Down Expand Up @@ -820,13 +819,6 @@ interface ISetProgramExtensions {
payload: Array<string>
}

interface IDeletedStalePageDataFiles {
type: `DELETED_STALE_PAGE_DATA_FILES`
payload: {
pagePathsToClear: Set<string>
}
}

interface IRemovedHtml {
type: `HTML_REMOVED`
payload: string
Expand Down
223 changes: 223 additions & 0 deletions packages/gatsby/src/services/__tests__/create-pages.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,223 @@
import { createPages } from "../create-pages"
import { store, emitter } from "../../redux"
import { actions } from "../../redux/actions"
import apiRunnerNode from "../../utils/api-runner-node"
import * as path from "path"

jest.mock(`../../utils/api-runner-node`)

jest.mock(`../../utils/js-chunk-names`, () => {
return { generateComponentChunkName: (): string => `--mocked--` }
})

let mockAPIs = {}

const component = path.join(process.cwd(), `wat`)

const testPlugin = {
name: `gatsby-source-test`,
version: `1.0.0`,
}

describe(`createPages service cleans up not recreated pages`, () => {
let RealDateNow
let DateNowCallCount = 0

let createPagesRun = 1
let createPagesStatefullyRun = 1

let createPagesHook
let createPagesStatefullyHook

let deletePageActions

function onDeletePage(deletePageAction): void {
deletePageActions.push(deletePageAction)
}

beforeAll(() => {
RealDateNow = Date.now
Date.now = jest.fn(() => ++DateNowCallCount)
apiRunnerNode.mockImplementation((apiName, opts = {}) => {
if (mockAPIs[apiName]) {
return mockAPIs[apiName](
{
actions: Object.keys(actions).reduce((acc, actionName) => {
acc[actionName] = (...args): any =>
store.dispatch(actions[actionName](...args, testPlugin, opts))
return acc
}, {}),
},
{}
)
}
return undefined
})

createPagesHook = mockAPIs[`createPages`] = jest.fn(
({ actions }, _pluginOptions) => {
actions.createPage({
path: `/stateless/stable`,
component,
})
actions.createPage({
path: `/stateless/dynamic/${createPagesRun}`,
component,
})
createPagesRun++
}
)

createPagesStatefullyHook = mockAPIs[`createPagesStatefully`] = jest.fn(
({ actions }, _pluginOptions) => {
actions.createPage({
path: `/stateful/stable`,
component,
})
actions.createPage({
path: `/stateful/dynamic/${createPagesStatefullyRun}`,
component,
})
createPagesStatefullyRun++
}
)

emitter.on(`DELETE_PAGE`, onDeletePage)
})

beforeEach(() => {
createPagesRun = 1
createPagesStatefullyRun = 1
createPagesHook.mockClear()
createPagesStatefullyHook.mockClear()
deletePageActions = []

store.dispatch({ type: `DELETE_CACHE` })
})

afterAll(() => {
Date.now = RealDateNow
mockAPIs = {}
emitter.off(`DELETE_PAGE`, onDeletePage)
})

it.each([
[`From cold cache`, { cacheStatus: `COLD` }],
[`From warm cache`, { cacheStatus: `WARM` }],
])(`%s`, async (_, { cacheStatus }) => {
expect(deletePageActions).toEqual([])
expect(store.getState().pages.size).toEqual(0)

if (cacheStatus === `WARM`) {
// add some junk
store.dispatch(
actions.createPage(
{
path: `/stateless/junk`,
component,
context: {},
},
testPlugin
)
)
store.dispatch(
actions.createPage(
{
path: `/stateful/junk`,
component,
context: {},
},
testPlugin,
{
traceId: `initial-createPagesStatefully`,
}
)
)

expect(store.getState().pages.size).toEqual(2)
expect(Array.from(store.getState().pages.keys())).toEqual([
`/stateless/junk`,
`/stateful/junk`,
])
expect(
store.getState().pages.get(`/stateless/junk`)
.isCreatedByStatefulCreatePages
).toEqual(false)
expect(
store.getState().pages.get(`/stateful/junk`)
.isCreatedByStatefulCreatePages
).toEqual(true)
} else {
expect(store.getState().pages.size).toEqual(0)
}

expect(mockAPIs[`createPages`]).toHaveBeenCalledTimes(0)
expect(mockAPIs[`createPagesStatefully`]).toHaveBeenCalledTimes(0)

await createPages({ store, shouldRunCreatePagesStatefully: true })

expect(mockAPIs[`createPages`]).toHaveBeenCalledTimes(1)
expect(mockAPIs[`createPagesStatefully`]).toHaveBeenCalledTimes(1)
expect(store.getState().pages.size).toEqual(4)
expect(Array.from(store.getState().pages.keys())).toEqual(
expect.arrayContaining([
`/stateless/stable`,
`/stateless/dynamic/1`,
`/stateful/stable`,
`/stateful/dynamic/1`,
])
)

if (cacheStatus === `WARM`) {
// "junk" pages were not recreated, so we expect DELETE_PAGE action to be emitted for those
expect(deletePageActions).toEqual(
expect.arrayContaining([
expect.objectContaining({
type: `DELETE_PAGE`,
payload: expect.objectContaining({
path: `/stateless/junk`,
}),
}),
])
)
expect(deletePageActions).toEqual(
expect.arrayContaining([
expect.objectContaining({
type: `DELETE_PAGE`,
payload: expect.objectContaining({
path: `/stateful/junk`,
}),
}),
])
)
}

await createPages({ store, shouldRunCreatePagesStatefully: false })

// createPagesStatefully should not be called and stateful pages should remain as they were before calling `createPages` service
expect(mockAPIs[`createPages`]).toHaveBeenCalledTimes(2)
expect(mockAPIs[`createPagesStatefully`]).toHaveBeenCalledTimes(1)
expect(store.getState().pages.size).toEqual(4)

expect(Array.from(store.getState().pages.keys())).toEqual(
expect.arrayContaining([
`/stateless/stable`,
`/stateless/dynamic/2`,
`/stateful/stable`,
`/stateful/dynamic/1`,
])
)

// 1st dynamic page was not recreated so we expect that we emitted DELETE_PAGE action
expect(deletePageActions).toEqual(
expect.arrayContaining([
expect.objectContaining({
type: `DELETE_PAGE`,
payload: expect.objectContaining({
path: `/stateless/dynamic/1`,
}),
}),
])
)
})
})
32 changes: 0 additions & 32 deletions packages/gatsby/src/services/create-pages-statefully.ts

This file was deleted.

0 comments on commit 7880240

Please sign in to comment.