Esbuild migration (minus testing stuff)#4894
Conversation
There was a problem hiding this comment.
We do this cleanReadme thing in a few extensions--should consider moving it into the eng package (probably just as a script that we run from node_modules)
There was a problem hiding this comment.
Pull request overview
Migrates the extension build pipeline from webpack/gulp to the @microsoft/vscode-azext-eng esbuild flow, aligning the codebase with NodeNext/ES2022 module semantics and updating related developer tooling.
Changes:
- Switch TypeScript config to
module/moduleResolution: nodenextandtarget/lib: es2022, plus ESM-friendly import updates across the codebase. - Replace gulp/webpack workflows with esbuild + new node-based helper scripts (e.g.,
pretest.mjs,cleanReadme.mjs) and update VS Code tasks/launch configs accordingly. - Update various tree items/types to work cleanly under the new build/type system (e.g.,
declarefields, icon path handling).
Reviewed changes
Copilot reviewed 49 out of 51 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Updates TS compilation settings for NodeNext/ES2022 and build output exclusions. |
| test/project/validateProject.ts | Switches to ESM-friendly globby import style. |
| test/index.ts | Updates imports to ESM-friendly defaults (globby/mocha/path). |
| src/utils/workspace.ts | Switches to ESM-friendly globby import style. |
| src/utils/treeUtils.ts | Changes icon path helpers to return Uri/IconPath instead of string paths. |
| src/utils/requestUtils.ts | Adjusts stream piping logic for downloads (introduces a regression). |
| src/utils/openUrl.ts | Switches to ESM-friendly open import style. |
| src/utils/azureClients.ts | Removes storage client helper in favor of newer azext exports. |
| src/tree/remoteProject/UserAssignedIdentitiesTreeItem.ts | Uses declare for parent typing under new TS emit behavior. |
| src/tree/remoteProject/SystemIdentityTreeItemBase.ts | Uses declare for parent typing under new TS emit behavior. |
| src/tree/remoteProject/RemoteFunctionsTreeItem.ts | Switches to ESM-friendly p-retry import and uses declare parent. |
| src/tree/remoteProject/RemoteFunctionTreeItem.ts | Uses declare for parent typing under new TS emit behavior. |
| src/tree/remoteProject/ManagedIdentityTreeItem.ts | Uses declare for parent typing under new TS emit behavior. |
| src/tree/localProject/LocalFunctionsTreeItem.ts | Uses declare for parent typing under new TS emit behavior. |
| src/tree/localProject/LocalFunctionTreeItem.ts | Uses declare for parent typing under new TS emit behavior. |
| src/tree/durableTaskScheduler/DurableTaskSchedulerResourceModel.ts | Reorders imports and switches to ESM-friendly p-retry import style. |
| src/tree/containerizedFunctionApp/ContainerFunctionsTreeItem.ts | Switches to ESM-friendly p-retry import style. |
| src/tree/containerizedFunctionApp/ContainerFunctionTreeItem.ts | Uses declare for parent typing under new TS emit behavior. |
| src/tree/SlotsTreeItem.ts | Uses declare for parent typing under new TS emit behavior. |
| src/tree/ResolvedFunctionAppResource.ts | Uses declare for data typing under new TS emit behavior. |
| src/tree/FunctionsTreeItemBase.ts | Uses declare for parent typing under new TS emit behavior. |
| src/tree/FunctionTreeItemBase.ts | Uses declare for parent typing under new TS emit behavior. |
| src/templates/script/ScriptTemplateProvider.ts | Switches to ESM-friendly extract-zip import style. |
| src/templates/script/PysteinTemplateProvider.ts | Uses declare for protected fields under new TS emit behavior. |
| src/extension.ts | Removes ignoreBundle param and aligns activation signature with new entrypoint behavior. |
| src/downloadAzureProject/setupProjectFolder.ts | Switches to ESM-friendly extract-zip import style. |
| src/constants-nls.ts | Adds license header (introduces BOM at file start). |
| src/commands/pickFuncProcess.ts | Updates ps-tree import/types and fixes minor formatting/semicolons. |
| src/commands/deploy/verifyAppSettings.ts | Switches to ESM-friendly p-retry import style. |
| src/commands/deploy/promptForEventGrid.ts | Switches to ESM-friendly p-retry import style. |
| src/commands/deploy/notifyDeployComplete.ts | Switches to ESM-friendly p-retry import style. |
| src/commands/createFunction/dotnetSteps/DotnetNamespaceStep.ts | Switches to ESM-friendly xregexp import style. |
| src/commands/createFunction/actionStepsV2/ActionSchemaStepBase.ts | Fixes initialization order by moving stepName assignment into constructor. |
| src/commands/createFunction/FunctionListStep.ts | Switches to ESM-friendly escape-string-regexp import style. |
| src/commands/appSettings/connectionSettings/azureWebJobsStorage/getStorageConnectionString.ts | Moves createStorageClient usage to azext-azureutils export. |
| src/commands/addBinding/settingSteps/StringPromptStep.ts | Uses declare for protected field typing under new TS emit behavior. |
| src/commands/addBinding/settingSteps/EnumPromptStep.ts | Uses declare for protected field typing under new TS emit behavior. |
| src/FunctionAppResolver.ts | Adds license header (introduces BOM at file start). |
| scripts/pretest.mjs | New node-based pretest script (replacing gulp pretest flow). |
| scripts/cleanReadme.mjs | New node-based README cleaning script (replacing gulp task). |
| package.json | Updates scripts/deps for esbuild migration and azext-eng tooling. |
| main.js | Updates runtime entrypoint to load the esbuild bundle (violates documented repo convention). |
| gulpfile.ts | Removes legacy gulp build/test helpers. |
| eslint.config.mjs | Adds flat-config style ESLint configuration via azext-eng. |
| esbuild.mjs | Adds esbuild entry script using azext-eng auto config. |
| .vscode/tasks.json | Updates default build tasks to esbuild + typecheck watch tasks. |
| .vscode/settings.json | Removes TS SDK/settings entries for workspace configuration. |
| .vscode/launch.json | Updates launch configs for esbuild output and new test config approach. |
| .vscode/extensions.json | Adds recommended esbuild problem matcher extension. |
| .nvmrc | Bumps Node version used by tooling/CI to 22. |
| const stream: NodeJS.ReadableStream = nonNullProp(response, 'readableStreamBody'); | ||
| await new Promise((resolve, reject): void => { | ||
| stream.pipe(fse.createWriteStream(filePath).on('finish', resolve).on('error', reject)); | ||
| stream.pipe(fse.createWriteStream(filePath).on('finish', () => resolve).on('error', reject)); |
There was a problem hiding this comment.
The promise in downloadFile never resolves because the finish handler returns the resolve function instead of calling it. Use resolve directly or call resolve() in the callback so downloads don't hang indefinitely.
| "lint": "eslint --max-warnings 0", | ||
| "lint-fix": "eslint --max-warnings 0 --fix", | ||
| "pretest": "npm run build && node scripts/pretest.mjs", | ||
| "test": "node ./out/test/runTest.js", |
There was a problem hiding this comment.
npm test runs node ./out/test/runTest.js, but the updated pretest/build scripts no longer emit compiled JS into out/ (only tsc --noEmit). On a clean CI agent this will fail because out/test/runTest.js won't exist. Either restore a build step that emits out/ for tests, or update the test script to match the new esbuild-based test runner.
| "test": "node ./out/test/runTest.js", | |
| "test": "vscode-test", |
| @@ -1,3 +1,7 @@ | |||
| /*--------------------------------------------------------------------------------------------- | |||
There was a problem hiding this comment.
This file now starts with a UTF-8 BOM character (visible before the opening /*). Please remove the BOM to avoid subtle tooling/linting issues and to match the encoding used by the rest of the codebase.
| /*--------------------------------------------------------------------------------------------- | |
| /*--------------------------------------------------------------------------------------------- |
| "--extensionTestsPath=${workspaceFolder}/out/test/index", | ||
| "${workspaceFolder}/test/test.code-workspace" | ||
| ], | ||
| "testConfiguration": "${workspaceFolder}/.vscode-test.mjs", |
There was a problem hiding this comment.
This launch config references ${workspaceFolder}/.vscode-test.mjs, but that file is not present in the repo. Add the missing test configuration file or update testConfiguration to point to the actual config so the "Launch Tests" profile works.
| "testConfiguration": "${workspaceFolder}/.vscode-test.mjs", |
| @@ -1,3 +1,7 @@ | |||
| /*--------------------------------------------------------------------------------------------- | |||
There was a problem hiding this comment.
This file now starts with a UTF-8 BOM character (visible before the opening /*). Please remove the BOM to avoid subtle tooling/linting issues and to match the encoding used by the rest of the codebase.
| /*--------------------------------------------------------------------------------------------- | |
| /*--------------------------------------------------------------------------------------------- |
* A whole lot of linting * Remove duplicate copyright notice Removed duplicate copyright notice from projectContextValues.ts * Fix duplicate license headers and update references to LICENSE.md (#4897) * Initial plan * Fix duplicate license headers and update license file references to LICENSE.md Co-authored-by: nturinski <5290572+nturinski@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: nturinski <5290572+nturinski@users.noreply.github.com> --------- Co-authored-by: Nathan Turinski <naturins@microsoft.comm> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: nturinski <5290572+nturinski@users.noreply.github.com>
|
@copilot Are you able to fix the build errors? |
|
@nturinski I've opened a new pull request, #4898, to work on those changes. Once the pull request is ready, I'll request review from you. |
Also need to get the need azuretools packages released, will handle that tomorrow.
Followed this handy dandy migration guide: https://github.com/microsoft/vscode-azuretools/blob/main/eng/MIGRATION.md
Used as reference: microsoft/vscode-azureresourcegroups@main...bmw/superEng
As well as: https://github.com/microsoft/vscode-azurestaticwebapps/pull/1010/files#diff-5e2d29a625d121212e4abfde1a84bcc47a121e5bf1fe7cc4135628deb89f4e73