Skip to content

Commit e7186ac

Browse files
bcomnesclaude
andcommitted
Address PR #215 review comments
- Export globalVarsNames, globalDataNames, esbuildSettingsNames, markdownItSettingsNames, templateSuffixes from identify-pages.js and import them in index.js — eliminates duplicated (and nodeHasTS-unaware) constant definitions in the watch handler - Add MAX_CONCURRENCY to index.js (consistent with build-pages/index.js) and use it in all three buildWatchMaps pMap calls instead of hardcoded 8 - findDepsOf now console.warn on failure instead of silently returning [] - Fix resolveVars JSDoc return type: always returns Promise<object>, not Promise<object|function>; remove stale type cast in buildWatchMaps - Merge BuildPagesOpts into DomStackOpts (pageFilterPaths, templateFilterPaths); drop the separate 5th buildPagesOpts parameter from buildPages/buildPagesDirect — worker.js now destructures opts - WorkerBuildStepResult is now a standalone typedef instead of Omit<PageBuildStepResult,'errors'> & {...} - Fix build-esbuild non-watch return to use extendedBuildOpts not buildOpts - Remove debug console.log('hello world!') from js-page/client.js example - Fix missing comma in .claude/settings.local.json Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 8bacbd7 commit e7186ac

9 files changed

Lines changed: 51 additions & 55 deletions

File tree

.claude/settings.local.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
{
22
"permissions": {
33
"allow": [
4-
"Bash(node --test:*)"
4+
"Bash(node --test:*)",
55
"WebFetch(domain:github.com)",
66
"WebFetch(domain:raw.githubusercontent.com)",
77
"Bash(gh repo view:*)",
8-
"Bash(gh api:*)"
8+
"Bash(gh api:*)",
9+
"mcp__mcp-server-github__add_reply_to_pull_request_comment",
10+
"mcp__mcp-server-github__pull_request_read"
911
]
1012
}
1113
}
Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1 @@
11
console.log('This client script runs on the js page')
2-
3-
console.log('hello world!')

index.js

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,10 @@
1010
* @import { PageInfo, TemplateInfo } from './lib/identify-pages.js'
1111
* @import { PageData } from './lib/build-pages/page-data.js'
1212
* @import { BuildOptions, BuildContext, BuildResult } from 'esbuild'
13-
* @import { BuildPagesOpts, PageBuildStepResult } from './lib/build-pages/index.js'
13+
* @import { PageBuildStepResult } from './lib/build-pages/index.js'
1414
*/
1515
import { once } from 'events'
16+
import { cpus } from 'os'
1617
import assert from 'node:assert'
1718
import chokidar from 'chokidar'
1819
import { basename, relative, resolve } from 'node:path'
@@ -30,7 +31,7 @@ import { getCopyGlob, buildStatic } from './lib/build-static/index.js'
3031
import { getCopyDirs, buildCopy } from './lib/build-copy/index.js'
3132
import { builder } from './lib/builder.js'
3233
import { buildPages } from './lib/build-pages/index.js'
33-
import { identifyPages } from './lib/identify-pages.js'
34+
import { identifyPages, globalVarsNames, globalDataNames, esbuildSettingsNames, markdownItSettingsNames, templateSuffixes } from './lib/identify-pages.js'
3435
import { buildEsbuild } from './lib/build-esbuild/index.js'
3536
import { ensureDest } from './lib/helpers/ensure-dest.js'
3637
import { resolveVars } from './lib/build-pages/resolve-vars.js'
@@ -91,6 +92,8 @@ import { DomStackAggregateError } from './lib/helpers/dom-stack-aggregate-error.
9192
* @typedef {PageData<T, U, V>} PageData
9293
*/
9394

95+
const MAX_CONCURRENCY = Math.min(cpus().length, 24)
96+
9497
const DEFAULT_IGNORES = /** @type {const} */ ([
9598
'.*',
9699
'coverage',
@@ -101,11 +104,6 @@ const DEFAULT_IGNORES = /** @type {const} */ ([
101104
'yarn.lock',
102105
])
103106

104-
const globalVarsNames = new Set(['global.vars.ts', 'global.vars.mts', 'global.vars.cts', 'global.vars.js', 'global.vars.mjs', 'global.vars.cjs'])
105-
const globalDataNames = new Set(['global.data.ts', 'global.data.mts', 'global.data.cts', 'global.data.js', 'global.data.mjs', 'global.data.cjs'])
106-
const esbuildSettingsNames = new Set(['esbuild.settings.ts', 'esbuild.settings.mts', 'esbuild.settings.cts', 'esbuild.settings.js', 'esbuild.settings.mjs', 'esbuild.settings.cjs'])
107-
const markdownItSettingsNames = new Set(['markdown-it.settings.ts', 'markdown-it.settings.mts', 'markdown-it.settings.cts', 'markdown-it.settings.js', 'markdown-it.settings.mjs', 'markdown-it.settings.cjs'])
108-
const templateSuffixes = ['.template.ts', '.template.mts', '.template.cts', '.template.js', '.template.mjs', '.template.cjs']
109107

110108
/**
111109
* Find transitive ESM dependencies of a file.
@@ -115,7 +113,8 @@ const templateSuffixes = ['.template.ts', '.template.mts', '.template.cts', '.te
115113
async function findDepsOf (filepath) {
116114
try {
117115
return await findDeps(filepath)
118-
} catch {
116+
} catch (err) {
117+
console.warn(`Warning: could not resolve deps of ${filepath}:`, err instanceof Error ? err.message : err)
119118
return []
120119
}
121120
}
@@ -154,7 +153,7 @@ async function buildWatchMaps (siteData) {
154153
if (!layoutDepMap.has(dep)) layoutDepMap.set(dep, new Set())
155154
layoutDepMap.get(dep)?.add(layout.layoutName)
156155
}
157-
}, { concurrency: 8 })
156+
}, { concurrency: MAX_CONCURRENCY })
158157

159158
// Build layoutPageMap, pageFileMap, and pageDepMap by resolving each page's layout var
160159
// and static dep analysis of its page file and page.vars file.
@@ -164,8 +163,8 @@ async function buildWatchMaps (siteData) {
164163
pageFileMap.set(pageInfo.pageFile.filepath, pageInfo)
165164
if (pageInfo.pageVars) pageFileMap.set(pageInfo.pageVars.filepath, pageInfo)
166165

167-
const pageVars = /** @type {Record<string, any>} */ (await resolveVars({ varsPath: pageInfo.pageVars?.filepath }).catch(() => ({})))
168-
const layoutName = /** @type {string} */ (pageVars['layout'] ?? 'root')
166+
const pageVars = await resolveVars({ varsPath: pageInfo.pageVars?.filepath }).catch(() => ({}))
167+
const layoutName = String((/** @type {Record<string, any>} */ (pageVars))['layout'] ?? 'root')
169168

170169
if (!layoutPageMap.has(layoutName)) layoutPageMap.set(layoutName, new Set())
171170
layoutPageMap.get(layoutName)?.add(pageInfo)
@@ -180,7 +179,7 @@ async function buildWatchMaps (siteData) {
180179
pageDepMap.get(dep)?.add(pageInfo)
181180
}
182181
}
183-
}, { concurrency: 8 })
182+
}, { concurrency: MAX_CONCURRENCY })
184183

185184
// Build templateDepMap via static dep analysis of each template file
186185
await pMap(siteData.templates, async (templateInfo) => {
@@ -189,7 +188,7 @@ async function buildWatchMaps (siteData) {
189188
if (!templateDepMap.has(dep)) templateDepMap.set(dep, new Set())
190189
templateDepMap.get(dep)?.add(templateInfo)
191190
}
192-
}, { concurrency: 8 })
191+
}, { concurrency: MAX_CONCURRENCY })
193192

194193
return { layoutDepMap, layoutPageMap, pageFileMap, layoutFileMap, pageDepMap, templateDepMap }
195194
}
@@ -298,12 +297,12 @@ export class DomStack {
298297
* @param {Set<TemplateInfo>} [templateFilter]
299298
*/
300299
const runPageBuild = async (pageFilter, templateFilter) => {
301-
/** @type {BuildPagesOpts} */
302-
const buildPagesOpts = {}
303-
if (pageFilter) buildPagesOpts.pageFilterPaths = [...pageFilter].map(p => p.pageFile.filepath)
304-
if (templateFilter) buildPagesOpts.templateFilterPaths = [...templateFilter].map(t => t.templateFile.filepath)
300+
/** @type {DomStackOpts} */
301+
const buildOpts = { ...opts }
302+
if (pageFilter) buildOpts.pageFilterPaths = [...pageFilter].map(p => p.pageFile.filepath)
303+
if (templateFilter) buildOpts.templateFilterPaths = [...templateFilter].map(t => t.templateFile.filepath)
305304
try {
306-
const pageBuildResults = await buildPages(src, dest, siteData, opts, buildPagesOpts)
305+
const pageBuildResults = await buildPages(src, dest, siteData, buildOpts)
307306

308307
buildLogger(pageBuildResults)
309308
return pageBuildResults
@@ -482,28 +481,28 @@ export class DomStack {
482481
// buildEsbuild() in the main process and passed to esbuild as `define` substitutions.
483482
// esbuild's own watcher does NOT track global.vars as an input, so any change could
484483
// affect bundle output and requires restarting esbuild with fresh `define` values.
485-
if (globalVarsNames.has(fileName)) {
484+
if (globalVarsNames.includes(fileName)) {
486485
console.log('global.vars changed, running full rebuild...')
487486
await fullRebuild()
488487
return
489488
}
490489

491490
// 2. global.data.* — data aggregation change, rebuild all pages
492-
if (globalDataNames.has(fileName)) {
491+
if (globalDataNames.includes(fileName)) {
493492
console.log('global.data changed, rebuilding all pages...')
494493
runPageBuild().catch(errorLogger)
495494
return
496495
}
497496

498497
// 3. esbuild.settings.* — full esbuild context restart + all pages
499-
if (esbuildSettingsNames.has(fileName)) {
498+
if (esbuildSettingsNames.includes(fileName)) {
500499
console.log('esbuild.settings changed, restarting esbuild...')
501500
await fullRebuild()
502501
return
503502
}
504503

505504
// 4. markdown-it.settings.* — rebuild all .md pages only (rendering change)
506-
if (markdownItSettingsNames.has(fileName)) {
505+
if (markdownItSettingsNames.includes(fileName)) {
507506
const mdPages = new Set(siteData.pages.filter(p => p.type === 'md'))
508507
logRebuildTree(fileName, mdPages)
509508
runPageBuild(mdPages).catch(errorLogger)

lib/build-esbuild/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ export async function buildEsbuild (src, dest, siteData, opts, watchOpts = {}) {
297297
report: {
298298
buildResults,
299299
outputMap,
300-
buildOpts,
300+
buildOpts: extendedBuildOpts,
301301
},
302302
}
303303
} catch (err) {

lib/build-pages/index.js

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -48,34 +48,27 @@ const __dirname = import.meta.dirname
4848
*/
4949

5050
/**
51-
* @typedef {Omit<PageBuildStepResult, 'errors'> & {
52-
* errors: {error: Error, errorData?: object}[]
53-
* }} WorkerBuildStepResult
51+
* @typedef {object} WorkerBuildStepResult
52+
* @property {'page'} type
53+
* @property {PageBuilderReport} report
54+
* @property {{error: Error, errorData?: object}[]} errors
55+
* @property {import('../builder.js').BuildStepWarnings} warnings
5456
*/
5557

5658
export { pageBuilders }
5759

58-
/**
59-
* Serializable version of build options passed through workerData.
60-
* Sets are not structured-clone serializable so we use filepath arrays.
61-
* @typedef {object} BuildPagesOpts
62-
* @property {string[]} [pageFilterPaths] - Only build pages whose pageFile.filepath is in this array
63-
* @property {string[]} [templateFilterPaths] - Only build templates whose templateFile.filepath is in this array
64-
*/
65-
6660
/**
6761
* Page builder glue. Most of the magic happens in the builders.
6862
* @param {string} src
6963
* @param {string} dest
7064
* @param {SiteData} siteData
71-
* @param {DomStackOpts} [_opts]
72-
* @param {BuildPagesOpts} [buildPagesOpts]
65+
* @param {DomStackOpts} [opts]
7366
* @returns {Promise<PageBuildStepResult>}
7467
*/
75-
export function buildPages (src, dest, siteData, _opts, buildPagesOpts = {}) {
68+
export function buildPages (src, dest, siteData, opts = {}) {
7669
return new Promise((resolve, reject) => {
7770
const worker = new Worker(join(__dirname, 'worker.js'), {
78-
workerData: { src, dest, siteData, buildPagesOpts },
71+
workerData: { src, dest, siteData, opts },
7972
})
8073

8174
worker.once('message', message => {
@@ -114,14 +107,14 @@ export function buildPages (src, dest, siteData, _opts, buildPagesOpts = {}) {
114107
* All layouts, variables and page builders need to resolve in here
115108
* so that it can be run more than once, after the source files change.
116109
*
117-
* @type {(...args: [...Parameters<PageBuildStep>, BuildPagesOpts?]) => Promise<WorkerBuildStepResult>} WorkerBuildStep
110+
* @type {(...args: Parameters<PageBuildStep>) => Promise<WorkerBuildStepResult>} WorkerBuildStep
118111
*/
119-
export async function buildPagesDirect (src, dest, siteData, _opts, buildPagesOpts = {}) {
120-
const pageFilterPaths = buildPagesOpts.pageFilterPaths
121-
? new Set(buildPagesOpts.pageFilterPaths)
112+
export async function buildPagesDirect (src, dest, siteData, opts = {}) {
113+
const pageFilterPaths = opts.pageFilterPaths
114+
? new Set(opts.pageFilterPaths)
122115
: null
123-
const templateFilterPaths = buildPagesOpts.templateFilterPaths
124-
? new Set(buildPagesOpts.templateFilterPaths)
116+
const templateFilterPaths = opts.templateFilterPaths
117+
? new Set(opts.templateFilterPaths)
125118
: null
126119

127120
/** @type {WorkerBuildStepResult} */

lib/build-pages/resolve-vars.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* @param {string} [params.varsPath] - Path to the file containing the variables.
66
* @param {object} [params.resolveVars] - Any variables you want passed to the reolveFunction.
77
* @param {string} [params.key='default'] - The key to extract from the imported module. Default: 'default'
8-
* @returns {Promise<object|function>} - Returns the resolved variables. If the imported variable is a function, it executes and returns its result. Otherwise, it returns the variable directly.
8+
* @returns {Promise<object>} - Returns the resolved variables. If the imported variable is a function, it executes and returns its result. Otherwise, it returns the variable directly.
99
*/
1010
export async function resolveVars ({
1111
varsPath,

lib/build-pages/worker.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ import { buildPagesDirect } from './index.js'
33

44
async function run () {
55
if (!parentPort) throw new Error('parentPort returned null')
6-
const { src, dest, siteData, buildPagesOpts } = workerData
6+
const { src, dest, siteData, opts } = workerData
77
let results
88
try {
9-
results = await buildPagesDirect(src, dest, siteData, {}, buildPagesOpts)
9+
results = await buildPagesDirect(src, dest, siteData, opts)
1010
parentPort.postMessage(results)
1111
} catch (err) {
1212
console.dir(results, { colors: true, depth: 999 })

lib/builder.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ import { ensureDest } from './helpers/ensure-dest.js'
5151
* @property {string[]|undefined} [target=[]] - Array of target strings to pass to esbuild
5252
* @property {boolean|undefined} [buildDrafts=false] - Build draft files with the published:false variable
5353
* @property {string[]|undefined} [copy=[]] - Array of paths to copy their contents into the dest directory
54+
* @property {string[]|undefined} [pageFilterPaths] - Internal: only build pages whose pageFile.filepath is in this array
55+
* @property {string[]|undefined} [templateFilterPaths] - Internal: only build templates whose templateFile.filepath is in this array
5456
*/
5557

5658
/**

lib/identify-pages.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,34 +70,36 @@ const globalClientNames = [
7070
'global.client.mjs', 'global.client.cjs'
7171
]
7272

73-
const globalVarsNames = nodeHasTS
73+
export const globalVarsNames = nodeHasTS
7474
? [
7575
'global.vars.ts', 'global.vars.mts', 'global.vars.cts',
7676
'global.vars.js', 'global.vars.mjs', 'global.vars.cjs'
7777
]
7878
: ['global.vars.js', 'global.vars.mjs', 'global.vars.cjs']
7979

80-
const globalDataNames = nodeHasTS
80+
export const globalDataNames = nodeHasTS
8181
? [
8282
'global.data.ts', 'global.data.mts', 'global.data.cts',
8383
'global.data.js', 'global.data.mjs', 'global.data.cjs'
8484
]
8585
: ['global.data.js', 'global.data.mjs', 'global.data.cjs']
8686

87-
const esbuildSettingsNames = nodeHasTS
87+
export const esbuildSettingsNames = nodeHasTS
8888
? [
8989
'esbuild.settings.ts', 'esbuild.settings.mts', 'esbuild.settings.cts',
9090
'esbuild.settings.js', 'esbuild.settings.mjs', 'esbuild.settings.cjs'
9191
]
9292
: ['esbuild.settings.js', 'esbuild.settings.mjs', 'esbuild.settings.cjs']
9393

94-
const markdownItSettingsNames = nodeHasTS
94+
export const markdownItSettingsNames = nodeHasTS
9595
? [
9696
'markdown-it.settings.ts', 'markdown-it.settings.mts', 'markdown-it.settings.cts',
9797
'markdown-it.settings.js', 'markdown-it.settings.mjs', 'markdown-it.settings.cjs'
9898
]
9999
: ['markdown-it.settings.js', 'markdown-it.settings.mjs', 'markdown-it.settings.cjs']
100100

101+
export const templateSuffixes = templateSuffixs
102+
101103
/**
102104
* Shape the file walker object
103105
*

0 commit comments

Comments
 (0)