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

feat(gatsby): lazy bundle page components in dev server #27884

Merged
merged 36 commits into from
Nov 17, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
b78ee23
feat(gatsby): lazy compile page components in dev server
KyleAMathews Nov 6, 2020
636afaf
Fix lint
KyleAMathews Nov 6, 2020
b46f680
Cleanup logs
KyleAMathews Nov 6, 2020
a47442e
cleanups
KyleAMathews Nov 6, 2020
a378040
fix tests
KyleAMathews Nov 6, 2020
a268a4d
Don't bother with navigating — if webpack doesn't update — the user p…
KyleAMathews Nov 6, 2020
4d07210
Update packages/gatsby/src/bootstrap/requires-writer.ts
KyleAMathews Nov 9, 2020
59a0385
Update packages/gatsby/cache-dir/dev-loader.js
KyleAMathews Nov 9, 2020
1cef9c0
Fix lint error
KyleAMathews Nov 9, 2020
56d5288
Wait until we know staticQueryHashes are written to page-data.json files
KyleAMathews Nov 10, 2020
29db87e
Disable check in production
KyleAMathews Nov 10, 2020
afc77cc
Only trigger compilation/page writing once
KyleAMathews Nov 10, 2020
237d715
Also pull xhr loaded static queries for <StaticQuery>
KyleAMathews Nov 10, 2020
d23f5fb
Merge branch 'master' into devjs-lazy
KyleAMathews Nov 10, 2020
f87a58c
instance isn't defined in builds apparently
KyleAMathews Nov 11, 2020
208cedf
Work around webpack hot reloading bug
KyleAMathews Nov 11, 2020
94aa19f
For static queries, data pushed from websocket is the most fresh
KyleAMathews Nov 11, 2020
902cf62
Centralize logic and prevent it from repeatedly hitting the backend
KyleAMathews Nov 12, 2020
4bca292
fix ordering of merge
KyleAMathews Nov 12, 2020
36738e3
Comment and add timeout to ensure page-data writes happen after the w…
KyleAMathews Nov 12, 2020
84fb3b0
Only add notInDevBundle during development
KyleAMathews Nov 12, 2020
2cf3a66
Add back mistakenly removed code
KyleAMathews Nov 12, 2020
c202cdf
Didn't need this
KyleAMathews Nov 12, 2020
6f55655
Implement @pieh's idea for telling runtime that dev bundle is loaded
KyleAMathews Nov 12, 2020
eb88597
Fix tests
KyleAMathews Nov 13, 2020
68236bf
Remove logs
KyleAMathews Nov 13, 2020
acfbe67
Merge branch 'master' into devjs-lazy
KyleAMathews Nov 16, 2020
a5fcd13
Put changes behind GATSBY_EXPERIMENT_LAZY_DEVJS flag
KyleAMathews Nov 16, 2020
53051e9
Add temporary fork of development-runtime tests for lazy devjs PR
KyleAMathews Nov 17, 2020
d140a1b
Remove duplicated tests w/ cleaner method @pieh suggested
KyleAMathews Nov 17, 2020
68efb11
Address @pieh feedback
KyleAMathews Nov 17, 2020
433918f
Update packages/gatsby/src/utils/start-server.ts
KyleAMathews Nov 17, 2020
fbd17a9
Remove global middleware
KyleAMathews Nov 17, 2020
3113e12
This isn't true
KyleAMathews Nov 17, 2020
78c0382
Merge branch 'master' into devjs-lazy
KyleAMathews Nov 17, 2020
34151f9
fix lint
KyleAMathews Nov 17, 2020
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
25 changes: 1 addition & 24 deletions packages/gatsby/cache-dir/__tests__/dev-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ describe(`Dev loader`, () => {
describe(`loadPage`, () => {
const createSyncRequires = components => {
return {
components,
lazyComponents: components,
}
}

Expand Down Expand Up @@ -375,29 +375,6 @@ describe(`Dev loader`, () => {
})
})

it(`should return an error when component cannot be loaded`, async () => {
const syncRequires = createSyncRequires({
chunk: false,
})
const devLoader = new DevLoader(syncRequires, [])
const pageData = {
path: `/mypage/`,
componentChunkName: `chunk`,
staticQueryHashes: [],
}
devLoader.loadPageDataJson = jest.fn(() =>
Promise.resolve({
payload: pageData,
status: `success`,
})
)

await devLoader.loadPage(`/mypage/`)
const expectation = devLoader.pageDb.get(`/mypage`)
expect(expectation).toHaveProperty(`status`, `error`)
expect(emitter.emit).toHaveBeenCalledTimes(0)
})

it(`should return an error pageData contains an error`, async () => {
const syncRequires = createSyncRequires({
chunk: `instance`,
Expand Down
6 changes: 4 additions & 2 deletions packages/gatsby/cache-dir/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import emitter from "./emitter"
import { apiRunner, apiRunnerAsync } from "./api-runner-browser"
import { setLoader, publicLoader } from "./loader"
import DevLoader from "./dev-loader"
import syncRequires from "$virtual/sync-requires"
import lazySyncRequires from "$virtual/lazy-client-sync-requires"
// Generated during bootstrap
import matchPaths from "$virtual/match-paths.json"

window.___emitter = emitter

const loader = new DevLoader(syncRequires, matchPaths)
window.lazySyncRequires = lazySyncRequires

const loader = new DevLoader(lazySyncRequires, matchPaths)
setLoader(loader)
loader.setApiRunner(apiRunner)

Expand Down
38 changes: 35 additions & 3 deletions packages/gatsby/cache-dir/dev-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,41 @@ import { BaseLoader, PageResourceStatus } from "./loader"
import { findPath } from "./find-path"

class DevLoader extends BaseLoader {
constructor(syncRequires, matchPaths) {
const loadComponent = chunkName =>
Promise.resolve(syncRequires.components[chunkName])
constructor(lazyRequires, matchPaths) {
// One of the tests doesn't set a path.
const loadComponent = chunkName => {
if (process.env.NODE_ENV !== `test`) {
delete require.cache[
require.resolve(`$virtual/lazy-client-sync-requires`)
]
lazyRequires = require(`$virtual/lazy-client-sync-requires`)
}
if (lazyRequires.lazyComponents[chunkName]) {
return Promise.resolve(lazyRequires.lazyComponents[chunkName])
} else {
return new Promise(resolve => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This promise doesn't reject (errors would be lost) and the timeout timer does not call reject nor resolve, meaning I think it ends up in a zombie state once it hits that path, since the interval that would call resolve and is the only way to resolve the promise is stopped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's nothing here to reject on I don't think. We're just telling the server to start compiling the page component and then... waiting. We're not doing anything per se.

Copy link
Contributor

Choose a reason for hiding this comment

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

Anything can crash. this.loadComponent can fail for any number of reasons 🤷

// Tell the server the user wants to visit this page
// to trigger it compiling the page component's code.
const req = new XMLHttpRequest()
req.open(`post`, `/___client-page-visited`, true)
req.setRequestHeader(`Content-Type`, `application/json;charset=UTF-8`)
req.send(JSON.stringify({ chunkName }))

const checkForUpdates = setInterval(() => {
if (process.env.NODE_ENV !== `test`) {
delete require.cache[
require.resolve(`$virtual/lazy-client-sync-requires`)
]
}
const lazyRequires = require(`$virtual/lazy-client-sync-requires`)
if (lazyRequires.lazyComponents[chunkName]) {
clearInterval(checkForUpdates)
resolve(lazyRequires.lazyComponents[chunkName])
}
}, 100)
KyleAMathews marked this conversation as resolved.
Show resolved Hide resolved
})
}
}
super(loadComponent, matchPaths)
}

Expand Down
1 change: 0 additions & 1 deletion packages/gatsby/cache-dir/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ const navigate = (to, options = {}) => {
})
}

console.log(`Site has changed on server. Reloading browser`)
window.location = pathname
}
}
Expand Down
11 changes: 10 additions & 1 deletion packages/gatsby/src/bootstrap/__tests__/requires-writer.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ describe(`requires-writer`, () => {
const program = {
directory: `/dir`,
}
let originalDateNow = global.Date.now
const clientVisitedPages = new Set()
const originalDateNow = global.Date.now

beforeEach(() => {
global.Date.now = () => now
Expand Down Expand Up @@ -61,6 +62,7 @@ describe(`requires-writer`, () => {
await requiresWriter.writeAll({
pages,
program,
clientVisitedPages,
})

expect(spy).toBeCalledWith(
Expand Down Expand Up @@ -108,6 +110,7 @@ describe(`requires-writer`, () => {
await requiresWriter.writeAll({
pages,
program,
clientVisitedPages,
})

expect(matchPaths[0].path).toBe(pages.get(`/app/login/`).path)
Expand Down Expand Up @@ -141,6 +144,7 @@ describe(`requires-writer`, () => {
await requiresWriter.writeAll({
pages,
program,
clientVisitedPages,
})

expect(matchPaths[0].path).toBe(pages.get(`/app/clients/static`).path)
Expand All @@ -165,6 +169,7 @@ describe(`requires-writer`, () => {
await requiresWriter.writeAll({
pages,
program,
clientVisitedPages,
})

expect(matchPaths[0].path).toBe(pages.get(`/`).path)
Expand Down Expand Up @@ -219,6 +224,7 @@ describe(`requires-writer`, () => {
await requiresWriter.writeAll({
pages,
program,
clientVisitedPages,
})

expect(matchPaths.map(p => p.matchPath)).toMatchInlineSnapshot(`
Expand All @@ -245,13 +251,15 @@ describe(`requires-writer`, () => {
await requiresWriter.writeAll({
pages,
program,
clientVisitedPages,
})

const matchPathsForInvertedInput = matchPaths
pages = generatePagesState(pagesInput)
await requiresWriter.writeAll({
pages,
program,
clientVisitedPages,
})

expect(matchPathsForInvertedInput).toEqual(matchPaths)
Expand All @@ -263,6 +271,7 @@ describe(`requires-writer`, () => {
await requiresWriter.writeAll({
pages,
program,
clientVisitedPages,
})

const allMatchingPages = matchPaths
Expand Down
86 changes: 42 additions & 44 deletions packages/gatsby/src/bootstrap/requires-writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,26 +162,41 @@ const getMatchPaths = (

const createHash = (
matchPaths: Array<IGatsbyPageMatchPath>,
components: Array<IGatsbyPageComponent>
components: Array<IGatsbyPageComponent>,
cleanedClientVisitedPageComponents: Array<IGatsbyPageComponent>
): string =>
crypto
.createHash(`md5`)
.update(JSON.stringify({ matchPaths, components }))
.update(
JSON.stringify({
matchPaths,
components,
cleanedClientVisitedPageComponents,
})
)
.digest(`hex`)

// Write out pages information.
export const writeAll = async (state: IGatsbyState): Promise<boolean> => {
// console.log(`on requiresWriter progress`)
const { program } = state
const pages = [...state.pages.values()]
const matchPaths = getMatchPaths(pages)
const components = getComponents(pages)
const clientVisitedPageComponents = [...state.clientVisitedPages.values()]

// Remove any page components that no longer exist.
const cleanedClientVisitedPageComponents = components.filter(c =>
clientVisitedPageComponents.some(s => s === c.componentChunkName)
)
pvdz marked this conversation as resolved.
Show resolved Hide resolved
KyleAMathews marked this conversation as resolved.
Show resolved Hide resolved

const newHash = createHash(matchPaths, components)
const newHash = createHash(
matchPaths,
components,
cleanedClientVisitedPageComponents
)

if (newHash === lastHash) {
// Nothing changed. No need to rewrite files
// console.log(`on requiresWriter END1`)
return false
}

Expand Down Expand Up @@ -211,6 +226,23 @@ const preferDefault = m => (m && m.default) || m
.join(`,\n`)}
}\n\n`

// Create file with sync requires of visited page components files.
let lazyClientSyncRequires = `${hotImport}
// prefer default export if available
const preferDefault = m => (m && m.default) || m
\n\n`
lazyClientSyncRequires += `exports.lazyComponents = {\n${cleanedClientVisitedPageComponents
.map(
(c: IGatsbyPageComponent): string =>
` "${
c.componentChunkName
}": ${hotMethod}(preferDefault(require("${joinPath(c.component)}")))`
)
.join(`,\n`)}
}\n\n`

writeModule(`$virtual/lazy-client-sync-requires`, lazyClientSyncRequires)

// Create file with async requires of components/json files.
let asyncRequires = `// prefer default export if available
const preferDefault = m => (m && m.default) || m
Expand Down Expand Up @@ -264,45 +296,11 @@ const preferDefault = m => (m && m.default) || m
return true
}

const debouncedWriteAll = _.debounce(
async (): Promise<void> => {
const activity = reporter.activityTimer(`write out requires`, {
id: `requires-writer`,
})
activity.start()
await writeAll(store.getState())
activity.end()
},
500,
{
// using "leading" can cause double `writeAll` call - particularly
// when refreshing data using `/__refresh` hook.
leading: false,
}
)

/**
* Start listening to CREATE/DELETE_PAGE events so we can rewrite
* Start listening to CREATE_CLIENT_VISITED_PAGE events so we can rewrite
* files as required
*/
export const startListener = (): void => {
emitter.on(`CREATE_PAGE`, (): void => {
reporter.pendingActivity({ id: `requires-writer` })
debouncedWriteAll()
})

emitter.on(`CREATE_PAGE_END`, (): void => {
reporter.pendingActivity({ id: `requires-writer` })
debouncedWriteAll()
})

emitter.on(`DELETE_PAGE`, (): void => {
reporter.pendingActivity({ id: `requires-writer` })
debouncedWriteAll()
})

emitter.on(`DELETE_PAGE_BY_PATH`, (): void => {
reporter.pendingActivity({ id: `requires-writer` })
debouncedWriteAll()
})
}
emitter.on(`CREATE_CLIENT_VISITED_PAGE`, (): void => {
reporter.pendingActivity({ id: `requires-writer` })
writeAll(store.getState())
})
13 changes: 13 additions & 0 deletions packages/gatsby/src/redux/actions/public.js
Original file line number Diff line number Diff line change
Expand Up @@ -1391,4 +1391,17 @@ actions.removePageData = (id: PageDataRemove) => {
}
}

/**
* Record that a page was visited in the Client..
*
* @param {Object} $0
* @param {string} $0.id the chunkName for the page component.
*/
actions.createClientVisitedPage = (chunkName: string) => {
return {
type: `CREATE_CLIENT_VISITED_PAGE`,
payload: { componentChunkName: chunkName },
}
}

module.exports = { actions }
29 changes: 29 additions & 0 deletions packages/gatsby/src/redux/reducers/client-visited-page.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import {
IGatsbyState,
IDeleteCacheAction,
ICreateClientVisitedPage,
} from "../types"

// The develop server always wants these page components.
const defaults = new Set<string>()
defaults.add(`component---cache-dev-404-page-js`)
defaults.add(`component---src-pages-404-js`)

export const clientVisitedPageReducer = (
state: IGatsbyState["clientVisitedPages"] = new Set<string>(defaults),
action: IDeleteCacheAction | ICreateClientVisitedPage
): IGatsbyState["clientVisitedPages"] => {
switch (action.type) {
case `DELETE_CACHE`:
return new Set<string>(defaults)

case `CREATE_CLIENT_VISITED_PAGE`: {
state.add(action.payload.componentChunkName)

return state
}

default:
return state
}
}
2 changes: 2 additions & 0 deletions packages/gatsby/src/redux/reducers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { schemaCustomizationReducer } from "./schema-customization"
import { inferenceMetadataReducer } from "./inference-metadata"
import { staticQueriesByTemplateReducer } from "./static-queries-by-template"
import { queriesReducer } from "./queries"
import { clientVisitedPageReducer } from "./client-visited-page"

/**
* @property exports.nodesTouched Set<string>
Expand All @@ -43,6 +44,7 @@ export {
configReducer as config,
schemaReducer as schema,
pagesReducer as pages,
clientVisitedPageReducer as clientVisitedPages,
statusReducer as status,
componentsReducer as components,
staticQueryComponentsReducer as staticQueryComponents,
Expand Down
7 changes: 7 additions & 0 deletions packages/gatsby/src/redux/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ export interface IGatsbyState {
}
pageDataStats: Map<SystemPath, number>
pageData: Map<Identifier, string>
clientVisitedPages: Set<string>
}

export interface ICachedReduxState {
Expand Down Expand Up @@ -589,6 +590,12 @@ interface ISetSchemaComposerAction {
payload: SchemaComposer<any>
}

export interface ICreateClientVisitedPage {
type: `CREATE_CLIENT_VISITED_PAGE`
payload: IGatsbyPage
plugin?: IGatsbyPlugin
}

export interface ICreatePageAction {
type: `CREATE_PAGE`
payload: IGatsbyPage
Expand Down
Loading