-
Notifications
You must be signed in to change notification settings - Fork 29
JS Packaging and bundling with ESBuild #8792
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
base: master
Are you sure you want to change the base?
Changes from all commits
97cfd62
71adb3c
fe467c9
f348e3f
ed5aa9e
4008d88
cbef713
aea8e11
1406071
e3a4d61
562d3f4
13efe94
3f94638
95c9c5d
baed423
50e792c
30e38a4
fa489fc
a9f802b
6bde645
2ed9654
1cb89c5
16d681a
ba39e02
42bcd80
2a1e6fb
2da3880
3c386c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,228 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const esbuild = require("esbuild"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const path = require("node:path"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const fs = require("node:fs"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const os = require("node:os"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const express = require("express"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const app = express(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const srcPath = path.resolve(__dirname, "frontend/javascripts/"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const outputPath = path.resolve(__dirname, "public/bundle/"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const protoPath = path.join(__dirname, "webknossos-datastore/proto"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Community plugins | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const browserslistToEsbuild = require("browserslist-to-esbuild"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const { lessLoader } = require("esbuild-plugin-less"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const copyPlugin = require("esbuild-plugin-copy").default; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const polyfillNode = require("esbuild-plugin-polyfill-node").polyfillNode; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const esbuildPluginWorker = require("@chialab/esbuild-plugin-worker").default; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const { wasmLoader } = require("esbuild-plugin-wasm"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Custom Plugins for Webknossos | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const { createWorkerPlugin } = require("./tools/esbuild/workerPlugin.js"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const { createProtoPlugin } = require("./tools/esbuild/protoPlugin.js"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const target = browserslistToEsbuild([ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"last 3 Chrome versions", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"last 3 Firefox versions", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"last 2 Edge versions", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"last 1 Safari versions", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"last 1 iOS versions", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function now() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const d = new Date(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return d.toISOString().replace("T", " ").replace("Z", ""); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
async function build(env = {}) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const isProduction = env.production || process.env.NODE_ENV === "production"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const isWatch = env.watch; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Determine output directory for bundles. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// In watch mode, it's a temp dir. In production, it's the public bundle dir. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const buildOutDir = isWatch | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
? path.join(os.tmpdir(), "webknossos-esbuild-dev") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
: outputPath; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
console.log("buildOutDir", buildOutDir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (isWatch) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Ensure a clean, stable directory for development builds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fs.rmSync(buildOutDir, { recursive: true, force: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fs.mkdirSync(buildOutDir, { recursive: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+51
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Temporary directory cleanup in watch mode could cause issues The code forcefully removes and recreates the temporary build directory on each watch mode start. This could cause issues if multiple instances are running or if the directory is in use. if (isWatch) {
// Ensure a clean, stable directory for development builds
- fs.rmSync(buildOutDir, { recursive: true, force: true });
- fs.mkdirSync(buildOutDir, { recursive: true });
+ try {
+ fs.rmSync(buildOutDir, { recursive: true, force: true });
+ fs.mkdirSync(buildOutDir, { recursive: true });
+ } catch (error) {
+ console.error(`Failed to prepare build directory: ${error.message}`);
+ throw error;
+ }
} 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let onBuildDone = () => {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Track ongoing builds so that incoming requests can be paused until all builds are finished. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let currentBuildCount = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let buildStartTime = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let buildCounterPlugin = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: 'buildCounterPlugin', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setup(build) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
build.onStart(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (currentBuildCount === 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
buildStartTime = performance.now(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
currentBuildCount++; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
console.log(now(), 'build started. currentBuildCount=', currentBuildCount) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
build.onEnd(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
currentBuildCount--; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (currentBuildCount === 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
console.log("Build took", performance.now() - buildStartTime); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
console.log(now(), 'build ended. currentBuildCount=', currentBuildCount) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (currentBuildCount === 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onBuildDone(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+60
to
+83
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Build tracking mechanism needs proper error handling The build counter plugin tracks concurrent builds but doesn't handle errors that might leave the counter in an inconsistent state. If a build fails catastrophically without triggering let buildCounterPlugin = {
name: 'buildCounterPlugin',
setup(build) {
build.onStart(() => {
if (currentBuildCount === 0) {
buildStartTime = performance.now();
}
currentBuildCount++;
console.log(now(), 'build started. currentBuildCount=', currentBuildCount)
})
- build.onEnd(() => {
+ build.onEnd((result) => {
currentBuildCount--;
if (currentBuildCount === 0) {
console.log("Build took", performance.now() - buildStartTime);
}
console.log(now(), 'build ended. currentBuildCount=', currentBuildCount)
+ // Log errors if present
+ if (result.errors.length > 0) {
+ console.error(`Build completed with ${result.errors.length} errors`);
+ }
if (currentBuildCount === 0) {
onBuildDone();
}
})
+ // Ensure counter is decremented even on dispose
+ build.onDispose(() => {
+ if (currentBuildCount > 0) {
+ console.warn('Build context disposed with active builds, resetting counter');
+ currentBuildCount = 0;
+ onBuildDone();
+ }
+ })
},
}
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const plugins = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
polyfillNode(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
createProtoPlugin(protoPath), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
wasmLoader(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lessLoader({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
javascriptEnabled: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
copyPlugin({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
patterns: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from: "node_modules/@zip.js/zip.js/dist/z-worker.js", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
to: path.join(buildOutDir, "z-worker.js"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
createWorkerPlugin({ logLevel: env.logLevel }), // Resolves import Worker from myFunc.worker; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
esbuildPluginWorker(), // Resolves new Worker(myWorker.js) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
buildCounterPlugin | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const buildOptions = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
entryPoints: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
main: path.resolve(srcPath, "main.tsx"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
bundle: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
outdir: buildOutDir, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
format: "esm", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
target: target, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
platform: "browser", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
splitting: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chunkNames: "[name].[hash]", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
assetNames: "[name].[hash]", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sourcemap: isProduction ? "external" : "inline", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+117
to
+118
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing cache busting for worker files The PR objectives mention "cache-busting for workers (emit hashed filenames or append nocache)" as a follow-up item. The current configuration generates hashed chunk names but workers might not benefit from this. The Would you like me to help implement cache-busted filenames for workers or create an issue to track this task? |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
minify: isProduction, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
define: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"process.env.NODE_ENV": JSON.stringify(isProduction ? "production" : "development"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"process.env.BABEL_ENV": JSON.stringify(process.env.BABEL_ENV || "development"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"process.browser": "true", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"global": "globalThis" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
loader: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".woff": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".woff2": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".ttf": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".eot": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".svg": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".png": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".jpg": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".jpeg": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".gif": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".webp": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".ico": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".mp4": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".webm": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
".ogg": "file", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
resolveExtensions: [".ts", ".tsx", ".js", ".json", ".proto", ".wasm"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
alias: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
react: path.resolve(__dirname, "node_modules/react"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
plugins, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
external: ["/assets/images/*", "fs"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logLevel: env.logLevel || "info", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
legalComments: isProduction ? "inline" : "none", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
publicPath: "/assets/bundle/", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
metafile: !isWatch, // Don't generate metafile for dev server | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logOverride: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"direct-eval": "silent", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (env.watch) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Development server mode | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const ctx = await esbuild.context(buildOptions); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const port = env.PORT || 9002; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// queue for waiting requests | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let waiting = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function gateMiddleware(req, res, next) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (currentBuildCount === 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return next(); // allow immediately | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// hold request | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
waiting.push({ req, res, next }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+165
to
+172
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Request queueing mechanism could cause memory issues The middleware queues all incoming requests when builds are in progress, which could lead to memory issues if many requests arrive during a long build. Consider adding a queue size limit or timeout: // queue for waiting requests
let waiting = [];
+const MAX_QUEUE_SIZE = 1000;
+const QUEUE_TIMEOUT_MS = 30000;
function gateMiddleware(req, res, next) {
if (currentBuildCount === 0) {
return next(); // allow immediately
}
+ if (waiting.length >= MAX_QUEUE_SIZE) {
+ res.status(503).send("Server too busy, please try again later");
+ return;
+ }
+
+ const timeout = setTimeout(() => {
+ const index = waiting.findIndex(w => w.req === req);
+ if (index !== -1) {
+ waiting.splice(index, 1);
+ res.status(504).send("Build timeout");
+ }
+ }, QUEUE_TIMEOUT_MS);
+
// hold request
- waiting.push({ req, res, next });
+ waiting.push({ req, res, next, timeout });
} 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
app.use("/", gateMiddleware, express.static(buildOutDir)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onBuildDone = function releaseRequests() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const queued = waiting; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
waiting = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
queued.forEach(({ next }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return next(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
app.listen(port, "127.0.0.1", () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
console.log(`Server running at http://localhost:${port}/assets/bundle/`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
console.log(`Development server running at http://localhost:${port}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
console.log(`Serving files from temporary directory: ${buildOutDir}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.on("SIGINT", async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await ctx.dispose(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.exit(0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+191
to
+194
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major SIGINT handler should handle cleanup errors The signal handler for graceful shutdown doesn't handle potential errors during context disposal. process.on("SIGINT", async () => {
+ console.log("\nShutting down development server...");
+ try {
await ctx.dispose();
+ console.log("Build context disposed successfully");
+ } catch (error) {
+ console.error("Error disposing build context:", error);
+ }
process.exit(0);
}); 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Production build | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const result = await esbuild.build(buildOptions); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (result.metafile) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await fs.promises.writeFile( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
path.join(buildOutDir, "metafile.json"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
JSON.stringify(result.metafile, null, 2) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
console.log("Build completed successfully!"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
module.exports = { build }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// If called directly | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (require.main === module) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const args = process.argv.slice(2); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const env = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logLevel: "info", // Default log level | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
args.forEach(arg => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (arg === "--production") env.production = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (arg === "--watch") env.watch = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (arg.startsWith("--port=")) env.PORT = Number.parseInt(arg.split("=")[1]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (arg === "--verbose") env.logLevel = "verbose"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (arg === "--silent") env.logLevel = "silent"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
build(env).catch(console.error); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modern browser targets may break compatibility
The browser targets are set to very recent versions (last 3 Chrome/Firefox, last 2 Edge, last 1 Safari/iOS). This aggressive targeting will drop support for older browsers and could affect users on enterprise or locked-down systems.
Based on the PR objectives mentioning "drops legacy browser support" as an expected impact, this appears intentional. However, ensure this aligns with your user base requirements. Consider documenting the minimum supported browser versions explicitly in your documentation.
The current configuration effectively requires:
If you need to support slightly older browsers, consider adjusting to:
📝 Committable suggestion
🤖 Prompt for AI Agents