build(vscode): remove bundled esbuild; require user-installed esbuild for config.ts #8016
Conversation
…y-sqlite.js and update imports
joffeoja
changed the title
dosubot
bot
added
the
size:L
|
Hi @Patrick-Erichsen and @RomneyDa, here is the reworked PR for removing the esbuild executable from the vscode extension along with the downloading logic, while keeping config.ts support (assuming end user installs esbuild themselves). The first upload had all tests pass but when I added the load.ts changes there were some fails. I don't see how they are related though 🤔 Edit: back to green. Let me know your thoughts on this third attempt at fixing the "vulnerable" esbuild situation. |
|
Hi @sestinj @RomneyDa & @Patrick-Erichsen, any update on this one? Would you like me to file an issue in order to track this work? |
| // Only bother with esbuild if config.ts is actually customized | ||
| if (currentContent.trim() !== DEFAULT_CONFIG_TS_CONTENTS.trim()) { | ||
| const ok = await handleEsbuildInstallation(ide, ideType); | ||
| if (!ok) { | ||
| // esbuild not available → we already showed a friendly message; skip building | ||
| return; | ||
| } | ||
| await tryBuildConfigTs(); | ||
| } |
There was a problem hiding this comment.
I think the goal from @sestinj 's feedback was just to provide the deprecation message to user's that have modified their config.ts , not to attempt to use their locally installed esbuild.
The general goal here is rip out all the config.ts logic but give a heads up to those affected.
Could we modify this to instead just show the toast with that in mind?
There was a problem hiding this comment.
@RomneyDa @Patrick-Erichsen @sestinj : The current code is based on feedback from Dallin in my now closed PR:
#7895 (review)
Spoke with the team further about this and we're thinking let's remove esbuild module installation while still supporting config.ts by assuming users already have esbuild installed and showing them an error with instructions if they don't e.g. config.ts has been deprecated and esbuild is no longer automatically installed by Continue. To use config.ts, you can run npm i [email protected] in ~/.continue`. Sorry for the partial bait and switch on this
As you requested I kept all the config.ts related logic, still letting the end user use config.ts if they have installed esbuild themselves. This PR leaves a deprecation message and the next logical step would be to remove all code pertaining to config.ts.
This PR supersedes my two earlier attempts:
#7790: Upgrade esbuild to less vulnerable version
#7895: Remove esbuild + config.ts support
There was a problem hiding this comment.
@joffeoja chatted with @Patrick-Erichsen, sounds like we'll move forward with this
Summary
Removes the bundled
esbuildbinary from the VS Code extension. The extension no longer shipsesbuild.exeor@esbuild/*platform packages in the.vsix. Build-timeesbuildusage is still retained.Details
Runtime esbuild removal
esbuildand@esbuild/*in prepackage scripts.esbuildfrom runtime dependencies..vscodeignoreentries to blockesbuildbinaries or*.exefrom slipping into the packaged VSIX.Build-time esbuild retained
scripts/esbuild.jsstill usesesbuildfor bundling during development/build.esbuildremains a devDependency.Config.ts behavior
config.tsis still supported but no longer auto-installsesbuild.config.tswithout havingesbuild, the extension shows a clear error with instructions:Other changes
download-copy-sqlite-esbuild.js→download-copy-sqlite.js.copyTreeSitterTagQryFileshelper.Verification
npm run packageproduces a.vsix.esbuildfiles in the VSIX (unzip -l continue-*.vsix | rg esbuild→ no hits).Summary by cubic
Stop bundling esbuild in the VS Code extension; esbuild is now used only at build time. This reduces the VSIX size and removes platform-specific binaries from runtime.
Refactors
Migration