Use esbuild to bundle #3834
Use esbuild to bundle #3834
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the build system from webpack to esbuild for the Prettier VS Code extension. The switch aims to improve build performance while maintaining the same functionality for both Node.js (desktop) and browser (web) extension builds.
Key Changes
- Replaced webpack configuration with a new
esbuild.jsbuild script - Updated npm scripts to use esbuild for building and watching
- Removed webpack-related dependencies (
webpack,webpack-cli,ts-loader,copy-webpack-plugin,vscode-nls-dev) - Added
esbuildandnpm-run-alldependencies
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| webpack.config.js | Removed entire webpack configuration file |
| esbuild.js | Added new esbuild build configuration with plugins for worker copying, browser aliasing, and problem matching |
| package.json | Updated scripts to use esbuild commands and replaced webpack dependencies with esbuild |
| src/ModuleResolver.ts | Fixed workaround for fs.statSync issue by using require() to get mutable reference |
| pnpm-workspace.yaml | Added esbuild to onlyBuiltDependencies list |
| pnpm-lock.yaml | Updated lockfile reflecting dependency changes |
| CLAUDE.md | Updated documentation to reflect esbuild usage |
| .vscode/tasks.json | Updated VS Code tasks for esbuild watching with proper problem matchers |
| .prettierignore | Added pnpm-lock.yaml to ignore list |
| "watch": "tsc --watch -p ./", | ||
| "webpack-dev": "webpack --mode development --watch", | ||
| "webpack": "webpack --mode development", | ||
| "chrome": "pnpm webpack && vscode-test-web --browserType=chromium --extensionDevelopmentPath=. .", |
There was a problem hiding this comment.
The 'chrome' script still references 'pnpm webpack' which no longer exists. This should be changed to 'pnpm compile' to use the new esbuild-based build.
| "chrome": "pnpm webpack && vscode-test-web --browserType=chromium --extensionDevelopmentPath=. .", | |
| "chrome": "pnpm compile && vscode-test-web --browserType=chromium --extensionDevelopmentPath=. .", |
| "webpack": "webpack --mode development", | ||
| "chrome": "pnpm webpack && vscode-test-web --browserType=chromium --extensionDevelopmentPath=. .", | ||
| "test:web": "pnpm test-compile && node ./out/test/web/runTests.js", | ||
| "prepare": "husky" |
There was a problem hiding this comment.
Missing 'vscode:prepublish' script which is required for VS Code extensions to build before publishing. This script should run the production build. Add: "vscode:prepublish": "node esbuild.js --production"
| "prepare": "husky" | |
| "prepare": "husky", | |
| "vscode:prepublish": "node esbuild.js --production" |
| name: "esbuild-problem-matcher", | ||
| setup(build) { | ||
| build.onStart(() => { | ||
| console.log("[watch] build started"); |
There was a problem hiding this comment.
[nitpick] The console.log in onStart should be conditional on watch mode. In non-watch mode, this message will appear unnecessarily. Consider checking the watch flag before logging.
| console.log("[watch] build started"); | |
| if (watch) { | |
| console.log("[watch] build started"); | |
| } |
| // Workaround for https://github.com/prettier/prettier-vscode/issues/3020 | ||
| // Use require() to get a mutable fs module reference and override statSync | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const mutableFs: Record<string, unknown> = require("fs"); |
There was a problem hiding this comment.
[nitpick] The type 'Record<string, unknown>' is too loose for the mutableFs variable. Consider using a more specific type like 'typeof fs & { statSync: typeof fs.statSync }' to maintain type safety while allowing mutation.
| const mutableFs: Record<string, unknown> = require("fs"); | |
| const mutableFs: typeof fs = require("fs"); |
Switches from webpack to esbuild.