-
Notifications
You must be signed in to change notification settings - Fork 35
Modernize build tooling: esbuild + pnpm and add multi-version CI testing #716
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: main
Are you sure you want to change the base?
Conversation
- Add esbuild.mjs build configuration - Update build/watch/package scripts to use esbuild - Enable caching for prettier and eslint commands - Remove webpack, ts-loader, and eslint-plugin-prettier
232ada8 to
0c0946b
Compare
- Bundle openpgp as CJS to avoid ESM runtime errors - Add type checking to build scripts - Fix iconPath to use vscode.Uri.file() - Update @types/node, @types/vscode, and VS Code engine to 1.95.0
- Run unit tests against Electron 32 and latest via matrix - Add integration test job for VS Code 1.95.0 and stable - Add test-electron.sh script for version-specific test runs
code-asher
left a comment
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.
Seems chill to me
| - name: Run tests with Electron ${{ matrix.electron-version }} | ||
| run: ./scripts/test-electron.sh ${{ matrix.electron-version }} | ||
| env: | ||
| CI: true | ||
|
|
||
| test-integration: | ||
| name: Integration Test (VS Code ${{ matrix.vscode-version }}) |
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.
Very nice indeed!
| }, | ||
|
|
||
| // Webpack config - CommonJS with Node globals | ||
| // Build config - ESM with Node globals |
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.
This could be me being dense but it looks like the esbuild.mjs outputs CJS, not ESM like implied in this comment?
Or is this just saying that the build config itself is ESM, not that the output of it is ESM?
| @@ -0,0 +1,9 @@ | |||
| # Native modules allowed to run install scripts (pnpm blocks by default for security) | |||
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.
Oh interesting, did not know about this. Definitely a plus for pnpm
Summary
This PR modernizes the build infrastructure, resulting in >2x faster CI builds through better caching (pnpm) and faster tooling (esbuild).
Build System
esbuild.mjsconfiguration with watch mode support and openpgp CJS alias fixPackage Manager Migration
--cache --cache-strategy content)CI/CD Improvements
pnpm/action-setup@v4with built-in cachingnpm install -g @vscode/vsce- use local devDependency insteadscripts/test-electron.shfor version-specific test runsOther Changes