-
-
Couldn't load subscription status.
- Fork 126
fix(vscode): try to load stdlib firstly from the installed zenstack package #2148
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
Conversation
📝 WalkthroughWalkthroughThe update refines the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ZModelWorkspaceManager
participant WorkspaceFolders
participant NodeModules
participant ExtensionResources
User->>ZModelWorkspaceManager: loadAdditionalDocuments()
ZModelWorkspaceManager->>WorkspaceFolders: For each folder, try to resolve zenstack/package.json
alt zenstack found in node_modules
ZModelWorkspaceManager->>NodeModules: Construct stdlib path in zenstack package
NodeModules-->>ZModelWorkspaceManager: Return stdlib path if exists
ZModelWorkspaceManager->>ZModelWorkspaceManager: Use resolved stdlib path
else zenstack not found
ZModelWorkspaceManager->>ExtensionResources: Use bundled stdlib path
end
ZModelWorkspaceManager->>ZModelWorkspaceManager: Load stdlib document
ZModelWorkspaceManager-->>User: Return loaded documents
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/schema/src/language-server/zmodel-workspace-manager.ts (3)
36-39: Replace blockingfs.existsSyncwith the async variant or Langium’s FS provider
fs.existsSyncblocks the event loop; in a VS Code extension this can delay the language server, especially in large multi-root workspaces. Using an asynchronous check (orthis.fileSystemProvider.exists(uri)) keeps the extension responsive.- if (fs.existsSync(candidateStdlibPath)) { + /* prefer async IO to avoid blocking the extension host */ + if (await fs.promises + .access(candidateStdlibPath, fs.constants.F_OK) + .then(() => true) + .catch(() => false)) {Alternatively, leverage Langium’s
fileSystemProvider.existsfor consistency with the rest of the codebase.
29-34: Guard against false-positive resolutions
require.resolve('zenstack/package.json', { paths: [folderPath] })throws any resolution error (including package.json missing). Consider checkingerror.code === 'MODULE_NOT_FOUND'before logging to avoid alarming stack traces for the normal “not found” case.- } catch (error) { + } catch (error: unknown) { + if ((error as NodeJS.ErrnoException)?.code !== 'MODULE_NOT_FOUND') { + console.error( + `Unexpected error while resolving zenstack in ${folder.uri}:`, + error + ); + }Reduces noisy logs while still surfacing genuine issues.
22-23: Nit:_folders/_collectorprefix implies “unused” but they are usedThe leading underscore conventionally signals an intentionally unused parameter. Since both variables are actively referenced, consider dropping the prefix to avoid confusion.
- protected async loadAdditionalDocuments( - _folders: WorkspaceFolder[], - _collector: (document: LangiumDocument) => void + protected async loadAdditionalDocuments( + folders: WorkspaceFolder[], + collector: (document: LangiumDocument) => voidAdjust internal references accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/schema/src/language-server/zmodel-workspace-manager.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/schema/src/language-server/zmodel-workspace-manager.ts
[error] 45-45: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build-test (20.x)
- GitHub Check: OSSAR-Scan
- GitHub Check: build-test (20.x)
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/schema/src/language-server/zmodel-workspace-manager.ts (1)
42-45: Drop the redundantcontinue; Biome still flags itThe
for … ofloop naturally proceeds to the next iteration after thecatchblock, so thecontinueis unnecessary and still triggersnoUnnecessaryContinue.- } catch (error) { - // Package not found or other error, continue to next folder - continue; - } + } catch (error) { + // Package not found or other error – move on to the next folder + }🧰 Tools
🪛 Biome (1.9.4)
[error] 44-44: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
🧹 Nitpick comments (2)
packages/schema/src/language-server/zmodel-workspace-manager.ts (2)
36-41: Replace blockingfs.existsSyncwith the async FS API
fs.existsSyncis synchronous and blocks the event-loop, which can hurt responsiveness inside the VS Code extension host. Use an async check (or just attempt toreadFile/statand swallowENOENT) instead.- if (fs.existsSync(candidateStdlibPath)) { - installedStdlibPath = candidateStdlibPath; - console.log(`Found installed zenstack package stdlib at ${installedStdlibPath}`); - break; - } + try { + await fs.promises.access(candidateStdlibPath, fs.constants.R_OK); + installedStdlibPath = candidateStdlibPath; + console.log(`Found installed zenstack package stdlib at ${installedStdlibPath}`); + break; + } catch { + /* not readable / not present – continue searching */ + }
30-35: Consider using the extension’s logger instead ofconsole.logRaw
console.logwrites go to the shared extension host output and can be noisy. If the project has a dedicated logging facility (e.g. VS CodeOutputChannelorconsolewrapper), prefer that for consistency and log-level control.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/schema/src/language-server/zmodel-workspace-manager.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/schema/src/language-server/zmodel-workspace-manager.ts
[error] 44-44: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: OSSAR-Scan
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: build-test (20.x)
- GitHub Check: Analyze (javascript-typescript)
No description provided.