Skip to content

refactor(workspace): hide workspace context - #154

Merged
9romise merged 6 commits into
mainfrom
workspace
Aug 29, 2026
Merged

refactor(workspace): hide workspace context#154
9romise merged 6 commits into
mainfrom
workspace

Conversation

@9romise

@9romise 9romise commented Aug 29, 2026

Copy link
Copy Markdown
Member

No description provided.

@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown

⚠️ Package Size Increase

📦 Package 📏 Base Size 📏 Source Size 📈 Size Change
npmx-language-server 238.1 kB 238.1 kB +1 B
npmx-language-service 9.2 kB 9.2 kB +1 B

@9romise
9romise marked this pull request as ready for review August 29, 2026 03:53
@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change replaces direct WorkspaceContext access with focused IWorkspaceState methods for catalog data, catalog dependencies, installed package manifests, and package engines. Catalog and installed-package plugins now use these methods. The engine-mismatch rule reads engines through the workspace interface. Test utilities provide the updated workspace shape, and a test verifies engine-mismatch diagnostics.

Merge Risk: 🟡 Moderate · up to f1e7c

The PR adds a Node-specific file-system dependency to browser-compatible source, which can break supported builds or runtimes; merge should wait until file-system access is routed through a host-specific adapter.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided. The title, “refactor(workspace): hide workspace context”, relates to the changes, but the missing description does not provide change details. Add a brief description that explains the workspace-context refactor, the new workspace lookup methods, and the affected language-service behaviour.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch workspace

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0262f249-b944-465f-9805-601b558bdd12

📥 Commits

Reviewing files that changed from the base of the PR and between 6dfbb80 and f1e7cd5.

📒 Files selected for processing (8)
  • packages/language-server/src/workspace.ts
  • packages/language-service/src/plugins/catalog.ts
  • packages/language-service/src/plugins/diagnostics/rules/__tests__/utils.ts
  • packages/language-service/src/plugins/diagnostics/rules/engine-mismatch.test.ts
  • packages/language-service/src/plugins/diagnostics/rules/engine-mismatch.ts
  • packages/language-service/src/plugins/installed-package-definition.test.ts
  • packages/language-service/src/plugins/installed-package-definition.ts
  • packages/language-service/src/types.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

import type { CatalogsInfo, Engines } from 'npmx-language-core/types'
import type { DependencyInfo, PackageManager, WorkspaceAdapter } from 'npmx-language-core/workspace'
import type { ClientFeatures, IWorkspaceState } from 'npmx-language-service/types'
import { access, realpath as fsRealpath, readFile } from 'node:fs/promises'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect browser-related package configuration and Node built-in imports in source.
fd -a -t f 'package.json' . -x sh -c '
  printf "\n== %s ==\n" "$1"
  rg -n "\"browser\"|\"exports\"|\"main\"|\"module\"|\"types\"" "$1" || true
' sh {} \;

rg -n --glob '*.{ts,tsx}' "from 'node:|from \"node:" packages/language-server/src

Repository: npmx-dev/vscode-npmx

Length of output: 1543


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== repository guidance =='
find /tmp/coderabbit-repo-knowledge/npmx-dev-vscode-npmx-74af7664 -type f -name '*.md' -print \
  | sort
printf '%s\n' '== package and source context =='
cat -n packages/language-server/package.json
sed -n '1,35p' packages/language-server/src/workspace.ts
printf '%s\n' '== matching source imports =='
rg -n --glob '*.{ts,tsx}' "from ['\"]node:" packages/language-server/src

Repository: npmx-dev/vscode-npmx

Length of output: 4070


Remove the Node built-in import from src/.

packages/language-server/src/workspace.ts imports node:fs/promises, which violates the repository’s browser-compatibility boundary for **/src/**/*.{ts,tsx}. Move file-system access behind a host-specific adapter.

Source: Coding guidelines

@9romise
9romise added this pull request to the merge queue Aug 29, 2026
Merged via the queue into main with commit 34931cd Aug 29, 2026
18 checks passed
@9romise
9romise deleted the workspace branch August 29, 2026 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant