Skip to content

Commit 29b332b

Browse files
Harden build pipeline per review feedback
- Use sass-embedded default export for sass-loader implementation - Move @babel/plugin-transform-runtime to plugins; add @babel/runtime dep - Validate BUILD_THREAD_WORKERS; cap to CPU; add PRODUCTION_POOL_TIMEOUT_MS - Handle watcher close errors under BUILD_WATCH_ONCE - Parse BUILD_CACHE_COMPRESSION strictly; warn on invalid values - Isolate webpack filesystem cache per variant (cache.name) - Fail on missing build artifacts in copyFiles (dev CSS placeholders allowed) - Make mkdir recursive for targetDir - Strengthen CI webpack cache key with Node version and build.mjs hash - Polish docs wording for Performance defaults These changes address concrete reviewer findings to prevent runtime errors, avoid cache collisions during parallel builds, stop silently missing artifacts, and make builds/CI more predictable and auditable.
1 parent b76041e commit 29b332b

File tree

4 files changed

+77
-24
lines changed

4 files changed

+77
-24
lines changed

.github/copilot-instructions.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ Always reference these instructions first and fall back to search or bash comman
3030
- Affects only `.cache/webpack` size/speed; does not change final artifacts
3131
- BUILD_WATCH_ONCE (dev): When set, `npm run dev` runs a single build and exits (useful for timing)
3232

33-
Performance defaults: esbuild is used for JS/CSS minification; dev injects CSS via style-loader,
34-
prod extracts CSS via MiniCssExtractPlugin; thread-loader is enabled by default in both dev and prod.
33+
Performance defaults: esbuild handles JS/CSS minification; in development CSS is injected via style-loader,
34+
in production CSS is extracted via MiniCssExtractPlugin; thread-loader is enabled by default in both dev and prod.
3535

3636
### Build Output Structure
3737

.github/workflows/pre-release-build.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ jobs:
3838
path: |
3939
.cache/webpack
4040
node_modules/.cache/webpack
41-
key: ${{ runner.os }}-webpack-${{ hashFiles('**/package-lock.json') }}
41+
key: ${{ runner.os }}-node20-webpack-${{ hashFiles('build.mjs') }}-${{ hashFiles('**/package-lock.json') }}
4242
restore-keys: |
43-
${{ runner.os }}-webpack-
43+
${{ runner.os }}-node20-webpack-
4444
- run: npm run build
4545

4646
- uses: josStorer/get-current-time@v2

build.mjs

Lines changed: 72 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,43 @@ const isWatchOnce = !!process.env.BUILD_WATCH_ONCE
1919
// Cache compression control: default none; allow override via env
2020
const cacheCompressionEnv = process.env.BUILD_CACHE_COMPRESSION
2121
let cacheCompressionOption
22-
if (cacheCompressionEnv) {
23-
const v = String(cacheCompressionEnv).toLowerCase()
24-
if (v === '0' || v === 'false' || v === 'none') cacheCompressionOption = false
25-
else if (v === 'brotli') cacheCompressionOption = 'brotli'
26-
else cacheCompressionOption = 'gzip' // treat any truthy/unknown as gzip
22+
if (cacheCompressionEnv != null) {
23+
const v = String(cacheCompressionEnv).trim().toLowerCase()
24+
if (v === '' || v === '0' || v === 'false' || v === 'none') {
25+
cacheCompressionOption = false
26+
} else if (v === 'gzip' || v === 'brotli') {
27+
cacheCompressionOption = v
28+
} else {
29+
console.warn(
30+
`[build] Unknown BUILD_CACHE_COMPRESSION="${cacheCompressionEnv}", defaulting to no compression`,
31+
)
32+
cacheCompressionOption = false
33+
}
2734
}
28-
const cpuCount = os.cpus()?.length || 1
29-
const envWorkers = process.env.BUILD_THREAD_WORKERS
35+
const cpuCountRaw = os.cpus()?.length
36+
const cpuCount = Number.isInteger(cpuCountRaw) && cpuCountRaw > 0 ? cpuCountRaw : 1
37+
const rawWorkers = process.env.BUILD_THREAD_WORKERS
3038
? parseInt(process.env.BUILD_THREAD_WORKERS, 10)
3139
: undefined
32-
const threadWorkers = Number.isInteger(envWorkers) && envWorkers > 0 ? envWorkers : cpuCount
40+
let threadWorkers
41+
if (Number.isInteger(rawWorkers) && rawWorkers > 0) {
42+
const maxWorkers = Math.max(1, cpuCount)
43+
if (rawWorkers > maxWorkers) {
44+
console.warn(
45+
`[build] BUILD_THREAD_WORKERS=${rawWorkers} exceeds CPU count (${cpuCount}); capping to ${maxWorkers}`,
46+
)
47+
}
48+
threadWorkers = Math.min(rawWorkers, maxWorkers)
49+
} else {
50+
if (process.env.BUILD_THREAD_WORKERS) {
51+
console.warn(
52+
`[build] Invalid BUILD_THREAD_WORKERS="${process.env.BUILD_THREAD_WORKERS}", defaulting to CPU count (${cpuCount})`,
53+
)
54+
}
55+
threadWorkers = cpuCount
56+
}
57+
// Thread-loader pool timeout constants
58+
const PRODUCTION_POOL_TIMEOUT_MS = 2000
3359
// Enable threads by default; allow disabling via BUILD_THREAD=0
3460
const enableThread = process.env.BUILD_THREAD === '0' ? false : true
3561

@@ -52,7 +78,16 @@ async function runWebpack(isWithoutKatex, isWithoutTiktoken, minimal, sourceBuil
5278
]
5379
if (isWithoutKatex) shared.push('./src/components')
5480

55-
const sassImpl = await import('sass-embedded')
81+
// Use the default export from sass-embedded; sass-loader expects the implementation object
82+
const { default: sassImpl } = await import('sass-embedded')
83+
84+
const variantId = [
85+
isWithoutKatex ? 'no-katex' : 'with-katex',
86+
isWithoutTiktoken ? 'no-tiktoken' : 'with-tiktoken',
87+
minimal ? 'minimal' : 'full',
88+
sourceBuildDir || outdir,
89+
isProduction ? 'prod' : 'dev',
90+
].join('|')
5691

5792
const compiler = webpack({
5893
entry: {
@@ -81,6 +116,7 @@ async function runWebpack(isWithoutKatex, isWithoutTiktoken, minimal, sourceBuil
81116
devtool: isProduction ? false : 'cheap-module-source-map',
82117
cache: {
83118
type: 'filesystem',
119+
name: `webpack-${variantId}`,
84120
// default none; override via BUILD_CACHE_COMPRESSION=gzip|brotli
85121
compression: cacheCompressionOption ?? false,
86122
buildDependencies: {
@@ -164,7 +200,11 @@ async function runWebpack(isWithoutKatex, isWithoutTiktoken, minimal, sourceBuil
164200
options: {
165201
workers: threadWorkers,
166202
// Ensure one-off dev build exits quickly
167-
poolTimeout: isProduction ? 2000 : isWatchOnce ? 0 : Infinity,
203+
poolTimeout: isProduction
204+
? PRODUCTION_POOL_TIMEOUT_MS
205+
: isWatchOnce
206+
? 0
207+
: Infinity,
168208
},
169209
},
170210
]
@@ -174,13 +214,9 @@ async function runWebpack(isWithoutKatex, isWithoutTiktoken, minimal, sourceBuil
174214
options: {
175215
cacheDirectory: true,
176216
cacheCompression: false,
177-
presets: [
178-
'@babel/preset-env',
179-
{
180-
plugins: ['@babel/plugin-transform-runtime'],
181-
},
182-
],
217+
presets: ['@babel/preset-env'],
183218
plugins: [
219+
['@babel/plugin-transform-runtime'],
184220
[
185221
'@babel/plugin-transform-react-jsx',
186222
{
@@ -309,7 +345,11 @@ async function runWebpack(isWithoutKatex, isWithoutTiktoken, minimal, sourceBuil
309345
else {
310346
const watching = compiler.watch({}, (err, stats) => {
311347
callback(err, stats)
312-
if (process.env.BUILD_WATCH_ONCE) watching.close(() => {})
348+
if (process.env.BUILD_WATCH_ONCE) {
349+
watching.close((closeErr) => {
350+
if (closeErr) console.error('Error closing watcher:', closeErr)
351+
})
352+
}
313353
})
314354
}
315355
}
@@ -325,11 +365,23 @@ async function zipFolder(dir) {
325365
}
326366

327367
async function copyFiles(entryPoints, targetDir) {
328-
if (!fs.existsSync(targetDir)) await fs.mkdir(targetDir)
368+
if (!fs.existsSync(targetDir)) await fs.mkdir(targetDir, { recursive: true })
329369
await Promise.all(
330370
entryPoints.map(async (entryPoint) => {
331-
if (await fs.pathExists(entryPoint.src)) {
332-
await fs.copy(entryPoint.src, `${targetDir}/${entryPoint.dst}`)
371+
try {
372+
if (await fs.pathExists(entryPoint.src)) {
373+
await fs.copy(entryPoint.src, `${targetDir}/${entryPoint.dst}`)
374+
} else {
375+
// Skip missing CSS in development (placeholders will be created later)
376+
const isCss = String(entryPoint.dst).endsWith('.css')
377+
if (!isProduction || isCss) {
378+
if (!isProduction && isCss) return
379+
}
380+
throw new Error(`Missing build artifact: ${entryPoint.src}`)
381+
}
382+
} catch (e) {
383+
console.error('Copy failed:', entryPoint, e)
384+
throw e
333385
}
334386
}),
335387
)

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
"lint"
2020
],
2121
"dependencies": {
22+
"@babel/runtime": "^7.24.7",
2223
"@mozilla/readability": "^0.6.0",
2324
"@nem035/gpt-3-encoder": "^1.1.7",
2425
"@picocss/pico": "^1.5.13",

0 commit comments

Comments
 (0)