fix: update ESM packaging and extension host loading#2735
fix: update ESM packaging and extension host loading#2735alisonlhart wants to merge 3 commits intomainfrom
Conversation
Fix language server loading in extension development host after ESM transition: - Add .js extension to vscode-languageserver/node import for ESM compatibility - Always bundle language server (fixes path alias issues in unbundled dev builds) - Use workspace server in debug mode (for Extension Development Host) In debug mode, the extension checks if the workspace server exists at packages/ansible-language-server/dist/cli.cjs and uses it. This allows the Extension Development Host to work when developing from source. Falls back to node_modules version (cli.js) if workspace server doesn't exist.
📝 WalkthroughWalkthroughThree files updated to modify import paths for the language server, enable unconditional code bundling in the build process, and add workspace-level debug server detection with filesystem checks for improved local development support. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/extension.ts (1)
1151-1190:⚠️ Potential issue | 🟠 Major
bundledServerandworkspaceServerare the identical path — dedupe and add arun-mode fallback.Lines 1153–1159 and 1162–1168 construct exactly the same path (
packages/ansible-language-server/dist/cli.cjs). Thefs.existsSync(workspaceServer)check at line 1169 only affectsdebug.module;run.moduleunconditionally usesbundledServer, so when the workspace build is absent (e.g., non-production tsup outputs tolib/, seepackages/ansible-language-server/tsup.config.ts), the non-debug code path will still point at a missing file with no fallback tonode_modules/@ansible/ansible-language-server/dist/cli.js.Also worth noting: the comment on line 1151 describes the old precedence ("Prefer the server shipped in the vsix; otherwise use the workspace package"), which no longer matches the new workspace-first/
node_modules-fallback behavior.♻️ Suggested consolidation
- // Prefer the server shipped in the vsix (packages/ansible-language-server/dist/); otherwise - // use the workspace package (e.g. when running from source). - const bundledServer = path.join( - context.extensionPath, - "packages", - "ansible-language-server", - "dist", - "cli.cjs", - ); - - // For debug mode: prefer workspace server (development) over node_modules (published package) - const workspaceServer = path.join( - context.extensionPath, - "packages", - "ansible-language-server", - "dist", - "cli.cjs", - ); - const packageServer = fs.existsSync(workspaceServer) - ? workspaceServer - : path.join( - context.extensionPath, - "node_modules", - "@ansible", - "ansible-language-server", - "dist", - "cli.js", - ); + // Prefer the workspace build (development from source) over the published + // package under node_modules (shipped in the VSIX via bundledDependencies). + const workspaceServer = path.join( + context.extensionPath, + "packages", + "ansible-language-server", + "dist", + "cli.cjs", + ); + const nodeModulesServer = path.join( + context.extensionPath, + "node_modules", + "@ansible", + "ansible-language-server", + "dist", + "cli.js", + ); + const resolvedServer = fs.existsSync(workspaceServer) + ? workspaceServer + : nodeModulesServer;…and then use
resolvedServerfor bothrun.moduleanddebug.module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/extension.ts` around lines 1151 - 1190, The two identical path variables bundledServer and workspaceServer should be consolidated and a single resolvedServer chosen with the correct precedence and fallback so both run.module and debug.module point at a valid file; create a single workspacePath (e.g., packages/ansible-language-server/dist/cli.cjs) and set resolvedServer = fs.existsSync(workspacePath) ? workspacePath : path.join(context.extensionPath, "node_modules", "@ansible", "ansible-language-server", "dist", "cli.js"), then use resolvedServer for serverOptions.run.module and serverOptions.debug.module (keeping debugOptions/execArgv as-is), and update the surrounding comment to accurately describe the chosen precedence and fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ansible-language-server/tsup.config.ts`:
- Around line 25-29: The outDir in tsup.config.ts switches to "lib" when
NODE_ENV !== "production" which breaks src/extension.ts and the run lookup that
expect packages/ansible-language-server/dist/cli.cjs; update the build config so
artifacts always go to "dist" (make outDir unconditionally "dist") or
alternatively update src/extension.ts and the run lookup to probe both
"dist/cli.cjs" and "lib/cli.cjs" (check functions/methods that build the path to
cli.cjs in src/extension.ts and the run command) so development builds placed in
lib/ are discovered.
---
Outside diff comments:
In `@src/extension.ts`:
- Around line 1151-1190: The two identical path variables bundledServer and
workspaceServer should be consolidated and a single resolvedServer chosen with
the correct precedence and fallback so both run.module and debug.module point at
a valid file; create a single workspacePath (e.g.,
packages/ansible-language-server/dist/cli.cjs) and set resolvedServer =
fs.existsSync(workspacePath) ? workspacePath : path.join(context.extensionPath,
"node_modules", "@ansible", "ansible-language-server", "dist", "cli.js"), then
use resolvedServer for serverOptions.run.module and serverOptions.debug.module
(keeping debugOptions/execArgv as-is), and update the surrounding comment to
accurately describe the chosen precedence and fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d48d4fe-c306-47f1-9124-9d6170d2ab8b
📒 Files selected for processing (3)
packages/ansible-language-server/src/server.tspackages/ansible-language-server/tsup.config.tssrc/extension.ts
| minify: env === "production", | ||
| bundle: env === "production", | ||
| bundle: true, | ||
| entry: ["src/**/*.ts"], | ||
| format: ["esm", "cjs"], | ||
| outDir: env === "production" ? "dist" : "lib", |
There was a problem hiding this comment.
outDir mismatch with extension host lookup path.
bundle is now unconditional, but outDir still switches to lib when NODE_ENV !== "production". Meanwhile src/extension.ts only looks for the workspace server at packages/ansible-language-server/dist/cli.cjs (and run has no fallback at all). Running a plain build/watch without NODE_ENV=production will place artifacts in lib/ and the Extension Development Host will silently fail to find dist/cli.cjs, defeating the PR's stated goal of enabling development from source.
Consider making outDir unconditionally dist, or update the extension to probe both dist/ and lib/.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ansible-language-server/tsup.config.ts` around lines 25 - 29, The
outDir in tsup.config.ts switches to "lib" when NODE_ENV !== "production" which
breaks src/extension.ts and the run lookup that expect
packages/ansible-language-server/dist/cli.cjs; update the build config so
artifacts always go to "dist" (make outDir unconditionally "dist") or
alternatively update src/extension.ts and the run lookup to probe both
"dist/cli.cjs" and "lib/cli.cjs" (check functions/methods that build the path to
cli.cjs in src/extension.ts and the run command) so development builds placed in
lib/ are discovered.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
github-project-automation
Bot
moved this from In Progress
to Done
in 🧰 devtools project board
1 participant
Fix language server loading in extension development host after ESM transition:
In debug mode, the extension checks if the workspace server exists at packages/ansible-language-server/dist/cli.cjs and uses it. This allows the Extension Development Host to work when developing from source. Falls back to node_modules version (cli.js) if workspace server doesn't exist.
This PR fixes language server loading in the Extension Development Host after the extension's transition to ESM. The changes add ESM compatibility, ensure the language server is always bundled to avoid path alias issues, and enable development-mode fallback to workspace builds.
vscode-languageserver/nodeimport tovscode-languageserver/node.jsfor ESM compatibility inserver.tstsup.config.tsto always bundle the language server, not just in productionextension.tsto check for development builds atpackages/ansible-language-server/dist/cli.cjs, with fallback to the publishednode_modulesversionrelated: #2682, #2498