Fix for windows build support #1106
Conversation
WalkthroughThe changes consolidate build scripts across multiple extensions by replacing shell conditionals with a centralized Node.js packaging script, swap shell-based directory creation with cross-platform mkdirp utility, update font icon character mappings, and migrate to a scoped npm package for font generation dependencies. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-11-25T06:34:10.812ZApplied to files:
📚 Learning: 2025-11-10T15:05:11.309ZApplied to files:
🔇 Additional comments (2)
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 (2)
workspaces/wso2-platform/wso2-platform-extension/package.json (1)
259-259:⚠️ Resolve Zod version mismatch between dependent packages in wso2-platform workspace.The wso2-platform-extension (which depends on
@wso2/wso2-platform-vscode-webviews) useszod@^3.22.4, while its dependency wso2-platform-webviews uses[email protected]—different major versions. Both import from"zod/v3", but the major version divergence in the same workspace dependency chain could cause issues. Align these to the same major version (either standardize on v3 or v4 across both packages).workspaces/ballerina/ballerina-extension/package.json (1)
1217-1217: Zod version inconsistency: ballerina-extension uses v4 while mi-extension and wso2-platform-extension use v3.Line 1217 sets
zod: ^4.1.8, but mi-extension uses^3.24.1and wso2-platform-extension uses^3.22.4. However, wso2-platform-webviews already uses4.1.11in this same codebase, so v4 compatibility is already established. The usage patterns in ballerina-extension (z.object, z.string, z.array, z.default, z.lazy, z.describe) are all v4-compatible—no deprecated APIs detected.Clarify whether the version split is intentional or if extensions should align to either v3 or v4 for consistency.
🧹 Nitpick comments (1)
workspaces/ballerina/ballerina-extension/package.json (1)
1172-1172: Postbuild script is functionally correct but complexity risks maintainability.The script correctly chains conditional
download-lslogic with unconditionalcopyFonts,copyJSLibs,package, andcopyVSIXsteps usingexecSyncwithstdio: 'inherit'. However, the inline Node.js implementation spans multiple commands in a single line, making it difficult to maintain, debug, and test.Recommendation: Extract this logic into a separate file (e.g.,
scripts/postbuild.js) for clarity and testability, especially since similar but simpler patterns are being applied in other extensions.Example refactored structure:
// scripts/postbuild.js const { execSync } = require('child_process'); const isPreRelease = String(process.env.isPreRelease).toLowerCase() === 'true'; try { // Conditional: download-ls or download-ls:prerelease execSync(isPreRelease ? 'pnpm run download-ls:prerelease' : 'pnpm run download-ls', { stdio: 'inherit' }); // Always run these in sequence execSync('pnpm run copyFonts', { stdio: 'inherit' }); execSync('pnpm run copyJSLibs', { stdio: 'inherit' }); execSync('pnpm run package', { stdio: 'inherit' }); execSync('pnpm run copyVSIX', { stdio: 'inherit' }); } catch (error) { process.exit(1); }Then update the script to:
"postbuild": "node scripts/postbuild.js"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
workspaces/ballerina/ballerina-extension/package.json(3 hunks)workspaces/ballerina/ballerina-extension/src/features/project/cmds/pack.ts(1 hunks)workspaces/choreo/choreo-extension/package.json(3 hunks)workspaces/common-libs/font-wso2-vscode/package.json(1 hunks)workspaces/common-libs/scripts/package-vsix.js(1 hunks)workspaces/mi/mi-extension/package.json(1 hunks)workspaces/wso2-platform/wso2-platform-extension/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/resources/icons/**/*.tsx : Create separate SVG icon components in src/resources/icons/ for all diagram icons and import them as React components
Applied to files:
workspaces/mi/mi-extension/package.jsonworkspaces/ballerina/ballerina-extension/package.json
📚 Learning: 2025-12-12T13:11:28.926Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 1096
File: workspaces/ballerina/ballerina-extension/src/features/project/cmds/add.ts:105-112
Timestamp: 2025-12-12T13:11:28.926Z
Learning: In workspaces/ballerina/ballerina-extension/src/features/project/cmds/add.ts, the `getPackage` function is only called in contexts where `projectInfo.children[].projectPath` is guaranteed to be available, so no filtering for undefined values is needed when mapping over children.
Applied to files:
workspaces/ballerina/ballerina-extension/src/features/project/cmds/pack.ts
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/**/*.config.{js,ts} : Use minimatch-compatible glob patterns for file matching in build and test configuration files
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/package.jsonworkspaces/ballerina/ballerina-extension/package.json
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/**/*.{ts,tsx} : Define all constants (node types, sizing, spacing) in src/resources/constants.ts and import them where needed instead of hardcoding values
Applied to files:
workspaces/ballerina/ballerina-extension/package.json
📚 Learning: 2025-11-10T15:05:11.309Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Applied to files:
workspaces/ballerina/ballerina-extension/package.json
🔇 Additional comments (8)
workspaces/ballerina/ballerina-extension/src/features/project/cmds/pack.ts (1)
68-68: Minor style consistency improvement.Adding the semicolon improves consistency with TypeScript/JavaScript conventions.
workspaces/common-libs/font-wso2-vscode/package.json (1)
15-15: Verify build script compatibility with Fantasticon 1.2.3.The Fantasticon 2.0.0+ versions have known Windows compatibility issues with glob 8.x, which justifies the downgrade to 1.2.3 for Windows build support. However, this is a major version downgrade (3.x → 1.x) that may introduce API differences.
Ensure that all build scripts (
gen-icons,build,copy-icons) remain compatible with version 1.2.3. Confirm that:
- The generate-font scripts in
workspaces/common-libs/font-wso2-vscode/src/generate-font/work with the older API- No Fantasticon 3.x-specific features are being used
- The configuration format matches version 1.2.3 expectations
Additionally, consider if the caret range
^1.2.3is appropriate—it will allow updates to 1.3.x but not 2.x. Verify this is intentional to avoid inadvertently pulling in future minor versions.workspaces/common-libs/scripts/package-vsix.js (1)
12-12: LGTM! Correct fix for Windows compatibility.Adding
shell: trueenables proper resolution ofnpxon Windows, where it exists asnpx.cmdornpx.ps1rather than a direct executable. This change correctly addresses the Windows build support issue described in the PR objectives.The security implications of
shell: trueare acceptable here since the command and arguments are fully controlled (no user input), and this is a build script rather than production code.workspaces/choreo/choreo-extension/package.json (2)
172-172: Script refactor for cross-platform directory creation is correct.The conversion from
mkdir -ptonode -e "require('fs').mkdirSync('./resources/jslibs', { recursive: true })"is a proper cross-platform equivalent that maintains the same behavior on Windows and Unix-like systems.
185-185: Extract inline Node.js script to a reusable module; identical pattern duplicated across extensions.The script is functionally correct with adequate error handling (execSync with
stdio: 'inherit'propagates errors naturally), but the inline implementation is identical inworkspaces/choreo/choreo-extension/package.json:185andworkspaces/wso2-platform/wso2-platform-extension/package.json:209.The codebase already demonstrates this extraction pattern—
workspaces/ballerina/ballerina-extension/scripts/download-ls.jsandworkspaces/common-libs/scripts/package-vsix.jsboth use similar logic for readingisPreRelease. Consider consolidating both extensions to use a shared script incommon-libsor a dedicatedscripts/package.jsfile within each extension to reduce duplication and improve maintainability.⛔ Skipped due to learnings
Learnt from: madushajg Repo: wso2/vscode-extensions PR: 868 File: workspaces/bi/bi-extension/src/utils.ts:224-242 Timestamp: 2025-11-10T15:05:11.309Z Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.Learnt from: madushajg Repo: wso2/vscode-extensions PR: 868 File: workspaces/bi/bi-extension/src/utils.ts:153-169 Timestamp: 2025-11-10T15:04:50.474Z Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.workspaces/wso2-platform/wso2-platform-extension/package.json (1)
196-196: Approved: Cross-platform script refactors are consistent with choreo-extension.Both the
copy-webview-libandpackagescripts use the same cross-platform approach seen in choreo-extension, ensuring consistent behavior across Windows and Unix-like systems.Also applies to: 209-209
workspaces/ballerina/ballerina-extension/package.json (2)
1004-1004: Verify icon glyph consistency across all extensions.Multiple icon character mappings are being updated:
distro-design-view:\f16e→\f19e(consistent with mi-extension and choreo-extension)distro-start:\f11a→\f214(unique to this extension)distro-debug:\f171→\f199(unique to this extension)distro-persist-diagram:\f133→\f1ef(unique to this extension)Ensure these mappings correspond to the correct glyphs in the regenerated
@wso2/font-wso2-vscodefont file.Also applies to: 1011-1011, 1018-1018, 1032-1032
1173-1173: Approved: copyJSLibs script correctly copies visualizer assets.The script properly chains two
copyfilescommands to copy JavaScript files from both ballerina-visualizer and trace-visualizer into the resources/jslibs directory.
| "default": { | ||
| "fontPath": "./resources/font-wso2-vscode/dist/wso2-vscode.woff", | ||
| "fontCharacter": "\\f16e" | ||
| "fontCharacter": "\\f19e" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for distro-design-view and distro-build-package icon mappings across all package.json files
rg -A 2 '"distro-design-view":|"distro-build-package":' --type jsonRepository: wso2/vscode-extensions
Length of output: 1038
🏁 Script executed:
#!/bin/bash
# Search for fontCharacter values in distro-design-view and distro-build-package sections
echo "=== distro-design-view fontCharacter values ==="
rg -B 2 -A 5 '"distro-design-view"' --type json | grep -A 5 fontCharacter | head -30
echo ""
echo "=== distro-build-package fontCharacter values ==="
rg -B 2 -A 5 '"distro-build-package"' --type json | grep -A 5 fontCharacter | head -30Repository: wso2/vscode-extensions
Length of output: 1229
🏁 Script executed:
#!/bin/bash
# Check distro-build-package in other extensions
echo "=== Checking all distro-build-package definitions ==="
rg -B 2 -A 5 '"distro-build-package"' --type json
echo ""
echo "=== Summary: fontCharacter values ==="
echo "distro-design-view across extensions:"
rg '"distro-design-view"' -A 10 --type json | grep fontCharacter
echo ""
echo "distro-build-package across extensions:"
rg '"distro-build-package"' -A 10 --type json | grep fontCharacterRepository: wso2/vscode-extensions
Length of output: 1139
Resolve fontCharacter inconsistency for distro-design-view across extensions.
The distro-design-view icon uses \f19e in mi-extension and ballerina-extension but \f133 in api-designer-extension. Verify whether this inconsistency is intentional or should be unified. The distro-build-package icon (\f179) is only defined in mi-extension and has no equivalent in other extensions.
3 participants
Purpose
Approach
fantasticon: Pinned thefantasticondependency to an earlier version forfont-wso2-vscodebuild stability.package-vsixExecution: Addedshell: trueto thespawnSyncoptions.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.