Skip to content

Add winapp node jsbindings for typed JS/TS WinRT bindings#536

Open
lei9444 wants to merge 37 commits into
mainfrom
leilzh/jsbindings
Open

Add winapp node jsbindings for typed JS/TS WinRT bindings#536
lei9444 wants to merge 37 commits into
mainfrom
leilzh/jsbindings

Conversation

@lei9444
Copy link
Copy Markdown
Contributor

@lei9444 lei9444 commented May 18, 2026

Description

Call modern Windows Runtime (WinRT) APIs directly from JS or TS inElectron / Node app — no native addon, no node-gyp / MSBuild step, full IntelliSense.

The CLI generates typed .js + .d.ts bindings for WinAppSDK (and any other WinRT) APIs from their .winmd metadata. Bindings call into WinRT at runtime via @microsoft/dynwinrt.

Usage Example

Bootstrap a new project or Add bindings to an existing project:

 npx winapp init --use-defaults    # interactive prompt defaults to Yes
image image

Related Issue

Type of Change

  • ✨ New feature

Checklist

  • New tests added for new functionality (if applicable)
  • Tested locally on Windows
  • Main README.md updated (if applicable)
  • docs/usage.md updated (if CLI commands changed)
  • Language-specific guides updated (if applicable)
  • Sample projects updated to reflect changes (if applicable)
  • Agent skill templates updated in docs/fragments/skills/ (if CLI commands/workflows changed)

Screenshots / Demo

Additional Notes

AI Description

This PR introduces typed JavaScript and TypeScript bindings for Windows Runtime (WinRT) APIs via the winapp node jsbindings command, enabling developers to directly call WinRT APIs without requiring native addons. Users can generate the bindings through a simplified setup process. New commands include:

npx winapp init --use-defaults

Developers can then opt to generate WinRT API bindings specifically for use in their JavaScript and TypeScript projects.

Copilot AI review requested due to automatic review settings May 18, 2026 06:45
@github-actions github-actions Bot added the enhancement New feature or request label May 18, 2026
Comment thread src/winapp-npm/scripts/generate-commands.mjs Fixed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new “JS/TS WinRT bindings” pipeline to WinAppCli, exposing it both as (1) npm-wrapper-only CLI surfaces (winapp init --js-bindings*, winapp node jsbindings ...) and (2) persisted workspace configuration via an opt-in jsBindings: block in winapp.yaml, backed by dynwinrt/dynwinrt-codegen orchestration and a winmd discovery lockfile.

Changes:

  • Introduces winapp node jsbindings add|generate commands and init --js-bindings* flags (npm-only gated via WINAPP_CLI_CALLER).
  • Adds new services/models for jsBindings config parsing/splicing, WinMD discovery/partitioning, lockfile IO, package manager detection, and package.json runtime-dependency injection.
  • Updates docs, CLI schema, samples, and adds extensive unit/integration test coverage for the new pipeline.

Reviewed changes

Copilot reviewed 66 out of 68 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/winapp-npm/src/winapp-commands.ts Exposes new init flags + node jsbindings add/generate through the npm programmatic API wrapper.
src/winapp-npm/src/cli.ts Adds node jsbindings to the npm shim’s command routing/help and forwards to the native CLI.
src/winapp-npm/scripts/generate-docs.mjs Improves JSDoc extraction to avoid truncation at @… and tweaks generated header.
src/winapp-npm/scripts/generate-commands.mjs Escapes problematic doc text and adds naming overrides for generated TS surface.
src/winapp-npm/README.md Documents node jsbindings add in the npm package readme.
src/winapp-npm/package.json Adds @microsoft/dynwinrt-codegen runtime dependency to the npm wrapper.
src/winapp-npm/package-lock.json Locks @microsoft/dynwinrt-codegen dependency and metadata.
src/winapp-CLI/WinApp.Cli/Services/YamlPackagesHasher.cs Adds stable hashing for packages: block for lockfile staleness checks.
src/winapp-CLI/WinApp.Cli/Services/WinmdsLockfileService.cs Adds best-effort read/write of .winapp/winmds.lock.json with schema validation and atomic writes.
src/winapp-CLI/WinApp.Cli/Services/UserPackageJsonService.cs Adds safe editing of user package.json to ensure runtime deps are production deps.
src/winapp-CLI/WinApp.Cli/Services/PackageManagerDetector.cs Detects npm/yarn/pnpm/bun for tailored install hints.
src/winapp-CLI/WinApp.Cli/Services/NpmWrapperVersionProvider.cs Locates npm wrapper’s pinned versions by walking up from the native exe.
src/winapp-CLI/WinApp.Cli/Services/JsBindingsPresets.cs Adds presets + per-package emit/ref/skip classification and winmd partitioning helpers.
src/winapp-CLI/WinApp.Cli/Services/IWorkspaceSetupService.cs Minor comment/doc cleanup.
src/winapp-CLI/WinApp.Cli/Services/IWinmdsLockfileService.cs New interface for winmd lockfile read/write.
src/winapp-CLI/WinApp.Cli/Services/IUserPackageJsonService.cs New interface + outcome enum for package.json injection.
src/winapp-CLI/WinApp.Cli/Services/IPackageManagerDetector.cs New interface for package manager detection.
src/winapp-CLI/WinApp.Cli/Services/INpmWrapperVersionProvider.cs New interface for wrapper-version resolution.
src/winapp-CLI/WinApp.Cli/Services/IJsBindingsWorkspaceService.cs New orchestration interface + context/result models for bindings pipeline.
src/winapp-CLI/WinApp.Cli/Services/IDynWinrtCodegenService.cs New interface for invoking dynwinrt-codegen.
src/winapp-CLI/WinApp.Cli/Services/IConfigService.cs Adds SaveJsBindingsOnly to splice jsBindings while preserving other YAML.
src/winapp-CLI/WinApp.Cli/Services/ConfigService.cs Extends YAML parsing/serialization to support jsBindings: and splicing logic.
src/winapp-CLI/WinApp.Cli/Models/WinmdsLockfile.cs Adds lockfile model + source-gen JSON context.
src/winapp-CLI/WinApp.Cli/Models/WinappConfig.cs Adds JsBindings optional config block to the main config model.
src/winapp-CLI/WinApp.Cli/Models/JsBindingsConfig.cs New model for user-facing jsBindings settings (lang/output/packages/extraTypes/refs).
src/winapp-CLI/WinApp.Cli/Helpers/HostBuilderExtensions.cs Registers new services/commands in DI and command tree.
src/winapp-CLI/WinApp.Cli/Commands/WinAppRootCommand.cs Adds node top-level command category/registration.
src/winapp-CLI/WinApp.Cli/Commands/NodeCommand.cs New node command host for npm-only subtrees.
src/winapp-CLI/WinApp.Cli/Commands/JsBindingsCommand.cs New node jsbindings command host with js-bindings alias.
src/winapp-CLI/WinApp.Cli/Commands/InitCommand.cs Adds --js-bindings* flags, preset alias flags, and npm-only gating.
src/winapp-CLI/WinApp.Cli/Commands/GenerateJsBindingsCommand.cs Implements node jsbindings generate (read-only yaml + codegen).
src/winapp-CLI/WinApp.Cli/Commands/AddJsBindingsCommand.cs Implements node jsbindings add (mutates yaml + codegen) with presets/force/no-prompt logic.
src/winapp-CLI/WinApp.Cli.Tests/YamlPackagesHasherTests.cs Unit tests for hash stability/case/ordering behavior.
src/winapp-CLI/WinApp.Cli.Tests/WorkspaceSetupServiceTests.cs Adds propagation tests for the jsBindings step within workspace setup.
src/winapp-CLI/WinApp.Cli.Tests/WinmdsLockfileServiceTests.cs Unit tests for lockfile schema/atomic write/read behavior and categorization.
src/winapp-CLI/WinApp.Cli.Tests/UserPackageJsonServiceTests.cs Unit tests for package.json injection outcomes + formatting preservation.
src/winapp-CLI/WinApp.Cli.Tests/TestDoubles/FakeJsBindingsWorkspaceService.cs Test double for orchestrator wiring tests.
src/winapp-CLI/WinApp.Cli.Tests/TestDoubles/FakeDynWinrtCodegenService.cs Test double for codegen invocation without spawning binaries.
src/winapp-CLI/WinApp.Cli.Tests/PackageManagerDetectorTests.cs Unit tests for package manager detection precedence.
src/winapp-CLI/WinApp.Cli.Tests/NpmWrapperVersionProviderTests.cs Unit tests for wrapper-version lookup and failure messaging.
src/winapp-CLI/WinApp.Cli.Tests/JsBindingsPresetsTests.cs Unit tests for presets, classification, partitioning, and scope demotion behavior.
src/winapp-CLI/WinApp.Cli.Tests/GenerateJsBindingsCommandTests.cs Command-tree and behavior tests for node jsbindings generate.
src/winapp-CLI/WinApp.Cli.Tests/DynWinrtCodegenStagingTests.cs Tests staging/swap semantics for safe output replacement.
src/winapp-CLI/WinApp.Cli.Tests/DynWinrtCodegenOutputSafetyTests.cs Tests output-dir safety gates (marker/reparse-point/escape rejection).
src/winapp-CLI/WinApp.Cli.Tests/DynWinrtCodegenInvocationTests.cs Tests executable resolution and safe PATH lookup behaviors.
src/winapp-CLI/WinApp.Cli.Tests/DynWinrtCodegenArgvTests.cs Tests argv building and helper list-merge/dedup logic.
src/winapp-CLI/WinApp.Cli.Tests/BaseCommandTests.cs Adds test DI stub for INpmWrapperVersionProvider to avoid layout walking.
scripts/generate-llm-docs.ps1 Updates schema version normalization and skill-command mapping.
samples/electron/test.Tests.ps1 Adds end-to-end smoke tests for jsBindings add flow in Electron sample.
README.md Adds node jsbindings add to root command list.
docs/usage.md Documents init jsBindings flags and node jsbindings add.
docs/npm-usage.md Updates generated npm API docs with new init flags and node jsbindings APIs.
docs/guides/electron/index.md Adds Electron guide path for dynwinrt JS bindings option.
docs/fragments/skills/winapp-cli/setup.md Adds skill snippet describing jsBindings usage.
docs/fragments/skills/winapp-cli/frameworks.md Adds framework guidance for jsBindings in Electron/npm usage.
docs/cli-schema.json Updates CLI schema to include new flags and node jsbindings command tree.
.gitignore Ignores local .claude/ directory.
.github/plugin/skills/winapp-cli/setup/SKILL.md Updates generated skill doc content (includes new init flags and unregister section).
.github/plugin/skills/winapp-cli/frameworks/SKILL.md Updates generated frameworks skill doc with jsBindings guidance.
.github/plugin/agents/winapp.agent.md Updates agent decision tree with jsBindings guidance for Electron.
Files not reviewed (1)
  • src/winapp-npm/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread README.md Outdated
Comment thread src/winapp-npm/README.md
Comment thread docs/usage.md Outdated
Comment thread scripts/generate-llm-docs.ps1 Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 18, 2026

Build Metrics Report

Binary Sizes

Artifact Baseline Current Delta
CLI (ARM64) 31.45 MB 31.51 MB 📈 +56.5 KB (+0.18%)
CLI (x64) 31.79 MB 31.85 MB 📈 +55.0 KB (+0.17%)
MSIX (ARM64) 13.22 MB N/A N/A
MSIX (x64) 14.05 MB N/A N/A
NPM Package 27.50 MB N/A N/A
NuGet Package 27.59 MB N/A N/A
VS Code Extension 20.32 MB N/A N/A

Test Results

0 passed out of 0 tests in 0.0s (-1193 tests, -459.5s vs. baseline)

Test Coverage

0% line coverage, 0% branch coverage · ⚠️ -17.2% vs. baseline

CLI Startup Time

35ms median (x64, winapp --version) · ✅ -10ms vs. baseline


Updated 2026-06-04 12:18:36 UTC · commit 0098260 · workflow run

Copy link
Copy Markdown
Member

@nmetulev nmetulev left a comment

Choose a reason for hiding this comment

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

🤖 Automated PR review by GitHub Copilot CLI (pr-review skill, Claude Opus 4.7 orchestrator + GPT-5.4 multi-model cross-check). Findings below are AI-generated suggestions; please verify before acting.

PR Review — leilzh/jsbindings vs origin/main (6 commits, 70 files, +10705/-157)

⚠️ This PR exceeds the 50-file soft guardrail. Full diff (~560 KB) was reviewed end-to-end across all 8 dimensions, but the sheer surface area means individual hunks got proportionally less attention than they would in a smaller PR.

Summary

Critical: 0 High: 1 Medium: 10 Low: 0 (plus 1 candidate-high dropped by multi-model cross-check — see below)

Coverage

Dimension Result
security ⚠ 2 findings
correctness ⚠ 3 findings
cli-ux ⚠ 3 findings (1 disputed by cross-check → dropped)
alternative-solution ⚠ 2 findings
test-coverage ⚠ 2 findings
docs-and-samples ⚠ 1 finding
packaging ✓ clean
multi-model ✓ 1/3 high confirmed, 1 downgraded to medium, 1 disputed; no new criticals/highs

Findings

ID Location Domain Summary
H1 DynWinrtCodegenService.cs:510-558 security Workspace-local node_modules/@microsoft/dynwinrt-codegen is preferred over the wrapper-bundled copy → a cloned repo can replace the binary winapp launches
M1 ConfigService.cs:44-560 alternative-solution YAML splicer/parser/renderer in ConfigService should be a WinappConfigDocument-style data type (cf. AppxManifestDocument)
M2 ConfigService.cs:454-459 correctness TryReadScalar keeps inline # comments in the value — output: bindings/winrt # generated parses as a literal #-suffixed path
M3 DynWinrtCodegenService.cs:54-60 cli-ux Resolved executable path logged at Information level; should be debug-only
M4 JsBindingsWorkspaceService.cs:15-906 alternative-solution 906-line "god service" with 11 injected deps mixes orchestration, discovery, config mutation, prompting, and package.json edits — over the soft size limit
M5 JsBindingsWorkspaceService.cs:329-345 test-coverage UNC hardening for additionalRefs and lockfile PartitionFromLockfile paths is uncovered (only additionalWinmds UNC is tested)
M6 JsBindingsWorkspaceService.cs:388-409 correctness Live NuGet dependency expansion swallows all exceptions silently → incomplete metadata graph can produce truncated bindings with no user warning
M7 JsBindingsWorkspaceService.cs:571-583 correctness Best-effort package.json update only catches InvalidOperationException; raw IOException/UnauthorizedAccessException from UserPackageJsonService will abort the command instead of degrading to a warning (downgraded from high)
M8 JsBindingsWorkspaceService.cs:767-797 test-coverage Overlap-aware old-output cleanup (the guard against --force --output deleting freshly-generated bindings) has no orchestration test
M9 UserPackageJsonService.cs:24-85 security EnsureRuntimeDependency writes package.json without rejecting reparse points → a malicious workspace can redirect the rewrite via a symlink/junction
M10 src/winapp-npm/src/cli.ts:202-214 cli-ux showCombinedHelp() is stale: doesn't list node jsbindings add / node jsbindings generate, so the new flow isn't discoverable from winapp --help
M11 src/winapp-VSC/package.json:18-120 docs-and-samples VS Code extension doesn't surface the new node jsbindings commands (or document the intentional product boundary)

Dropped after multi-model cross-check

  • H3 (cli-ux): --config-dir doesn't retarget the workspace for node jsbindings add. Disputed → dropped. Independent re-read shows base-directory (workspace) and --config-dir (location of winapp.yaml) are intentionally separable; AddJsBindings_WithConfigDirSeparateFromWorkspace_PatchesIntendedYaml locks this in, and the docs describe it that way. This is at most a docs/ergonomics conversation, not a correctness bug.

Details

H1 src/winapp-CLI/WinApp.Cli/Services/DynWinrtCodegenService.cs:510-558

  • Severity: high · Confidence: high · Domain: security · Multi-model: confirmed
  • Finding: ResolveCodegenInvocation executes a workspace-local node_modules/@microsoft/dynwinrt-codegen before the wrapper-bundled copy, so a cloned repo (or anything that drops a fake under the user's workspace) can replace the binary winapp launches.
  • Evidence: Lines 510-514 explicitly prioritize a repo-controlled install (// Search workspace ancestry first (user-installed override)..., var roots = new List<DirectoryInfo> { workspaceDir };). Lines 533-558 return and execute that package immediately. The dedicated test ResolveCodegenInvocation_InnerNodeModulesShadowsOuter locks in the workspace-local precedence.
  • Recommendation: Resolve codegen from the wrapper's own pinned install first (or exclusively), and only honor a workspace override behind an explicit opt-in flag with version/integrity validation.

M1 src/winapp-CLI/WinApp.Cli/Services/ConfigService.cs:44-560

  • Severity: medium · Confidence: high · Domain: alternative-solution
  • Finding: ConfigService now owns a full custom YAML parser/splicer/renderer for jsBindingsSaveJsBindingsOnly(), Parse() / ParseJsBindingsLine(), AppendJsBindingsBlock(), SpliceJsBindingsBlock(). That's a data-document responsibility, not file I/O.
  • Evidence: Lines 44-65 (splice), 172-452 (parse), 513-560 (render) all live inside the service.
  • Recommendation: Extract winapp.yaml structure handling into a WinappConfigDocument / JsBindingsConfigDocument (mirroring AppxManifestDocument), keeping ConfigService as a thin load/save wrapper. Otherwise this file becomes the long-term home for ad-hoc YAML grammar.

M2 src/winapp-CLI/WinApp.Cli/Services/ConfigService.cs:454-459

  • Severity: medium · Confidence: high · Domain: correctness
  • Finding: TryReadScalar only strips leading prefix and surrounding quotes; an unquoted trailing # comment is kept as part of the value. So output: bindings/winrt # generated files parses as bindings/winrt # generated files.
  • Evidence: value = t.Substring(prefix.Length).Trim().Trim('"', '\''); (lines 454-459). ParseJsBindingsLine routes both lang and output through this helper (lines 268-269).
  • Recommendation: Strip everything from the first unquoted # (and apply the same rule to list items) before trimming quotes; add a regression test with a commented scalar.

M3 src/winapp-CLI/WinApp.Cli/Services/DynWinrtCodegenService.cs:54-60

  • Severity: medium · Confidence: high · Domain: cli-ux
  • Finding: Default-verbosity runs print the resolved dynwinrt-codegen executable at Information, exposing internal node_modules / binary paths that should be debug-only (and which already get added to taskContext.AddDebugMessage on the next line).
  • Evidence: logger.LogInformation("Resolved dynwinrt-codegen → {Executable} {PrefixArgs}", ...) (lines 54-60).
  • Recommendation: Downgrade to LogDebug; the debug-message line below already preserves it for --verbose.

M4 src/winapp-CLI/WinApp.Cli/Services/JsBindingsWorkspaceService.cs:15-906

  • Severity: medium · Confidence: high · Domain: alternative-solution
  • Finding: 906-line service with 11 constructor-injected dependencies combines orchestration (RunAsync, AddAsync, GenerateAsync), winmd discovery (ResolveAdditionalWinmds), interactive prompting, runtime dependency editing (EnsureRuntimeDependencyAndPrintHint), and config mutation. Well over the repo's soft ~800-line limit and the "one responsibility" guideline in the agent docs.
  • Evidence: Lines 15-27 (11 deps), 29-155 (RunAsync), 415-493 (winmd discovery), 554-613 (runtime dep), 615-809 (AddAsync), 811-880 (GenerateAsync).
  • Recommendation: Keep top-level orchestration here; extract winmd-discovery + runtime-dependency editing + config-prompt path into separate collaborators (or partials). Brings the file back under soft limit and narrows the surface for future changes.

M5 src/winapp-CLI/WinApp.Cli/Services/JsBindingsWorkspaceService.cs:329-345

  • Severity: medium · Confidence: high · Domain: test-coverage
  • Finding: UNC/network-path hardening is only half-tested: there's a test for additionalWinmds UNC rejection, but no companion for additionalRefs, and PartitionFromLockfile's UNC drop branch has no unit test either.
  • Evidence: Production code drops UNC paths in both emit and ref sets (if (IsNetworkPath(path)) continue; in lockfile partition, jsBindings.{FieldName} entry refused for additionalRefs/additionalWinmds). Coverage in AddJsBindingsOrchestrationTests only exercises additionalWinmds; WinmdsLockfileServiceTests doesn't feed UNC paths through PartitionFromLockfile.
  • Recommendation: Add (a) an orchestration test with jsBindings.additionalRefs pointing at a UNC path, (b) a unit test feeding UNC paths through PartitionFromLockfile for both emit and ref-only packages, asserting they're dropped before any FileInfo probing.

M6 src/winapp-CLI/WinApp.Cli/Services/JsBindingsWorkspaceService.cs:388-409

  • Severity: medium · Confidence: high · Domain: correctness
  • Finding: Live-mode transitive expansion swallows every NuGet lookup exception silently and continues with a partial dependency graph; downstream packageLayoutService.FindWinmds(usedVersions) then generates bindings from incomplete metadata with no user-visible warning.
  • Evidence: var deps = await nugetService.GetPackageDependenciesAsync(...) wrapped by catch (Exception ex) { taskContext.AddDebugMessage(...); logger.LogDebug(...); } (lines 390-409), feeding into LiveDiscoveryAsync at 254-267.
  • Recommendation: Surface transitive-expansion failures as a user-visible warning/error (or fail the slow path with "run winapp restore first"), rather than producing silently truncated bindings.

M7 src/winapp-CLI/WinApp.Cli/Services/JsBindingsWorkspaceService.cs:571-583

  • Severity: medium · Confidence: high · Domain: correctness · Multi-model: downgrade from high (real bug, requires abnormal FS state to trigger)
  • Finding: EnsureRuntimeDependencyAndPrintHint only catches InvalidOperationException, but UserPackageJsonService.EnsureRuntimeDependency performs raw File.OpenRead / File.ReadAllText / File.WriteAllText that can throw IOException / UnauthorizedAccessException. A locked or read-only package.json will abort init --js-bindings / node jsbindings * instead of degrading to the documented manual-install warning.
  • Evidence: Caller catches InvalidOperationException only (lines 571-583); callee raw I/O at UserPackageJsonService.cs:35-36,79-85. Same exception bubbles out of both init and post-codegen paths.
  • Recommendation: Wrap filesystem exceptions inside UserPackageJsonService.EnsureRuntimeDependency (return a failure outcome), or broaden the caller's catch to include IOException / UnauthorizedAccessException. Don't blanket-catch Exception.

M8 src/winapp-CLI/WinApp.Cli/Services/JsBindingsWorkspaceService.cs:767-797

  • Severity: medium · Confidence: high · Domain: test-coverage
  • Finding: The overlap-aware old-output cleanup — the guard that prevents --force --output from deleting freshly generated bindings when old/new directories nest — has no orchestration test.
  • Evidence: Code now skips cleanup when IsNestedPath is true in either direction and logs "Previous bindings dir {OldDir} overlaps new output {NewDir}; skipping cleanup.". Orchestration tests in AddJsBindingsOrchestrationTests.cs only use disjoint dirs (managed-old/fresh-out, unmanaged-old/fresh-out-2).
  • Recommendation: Add cases for both nesting directions (bindingsbindings/winrt and the reverse), asserting cleanup is skipped and freshly generated files survive.

M9 src/winapp-CLI/WinApp.Cli/Services/UserPackageJsonService.cs:24-85

  • Severity: medium · Confidence: high · Domain: security
  • Finding: EnsureRuntimeDependency builds workspaceDirectory + "package.json", checks existence, then unconditionally File.OpenRead / File.WriteAllTexts — no reparse-point or root-containment check. A malicious workspace can redirect the automatic dependency rewrite via a symlink/junction to a package.json outside the workspace.
  • Evidence: Lines 35-36 (File.OpenRead) and 79-85 (File.WriteAllText) with only File.Exists between them.
  • Recommendation: Reject the rewrite when FileInfo(packageJsonPath).Attributes.HasFlag(FileAttributes.ReparsePoint) (or any ancestor is a reparse point), or canonicalize and require the resolved path to stay inside the workspace root.

M10 src/winapp-npm/src/cli.ts:202-214

  • Severity: medium · Confidence: high · Domain: cli-ux
  • Finding: showCombinedHelp() is stale: it still lists only node create-addon, node add-electron-debug-identity, node clear-electron-debug-identity. The new winapp node jsbindings add / generate flow only appears in the nested winapp node help, not in the top-level winapp --help listing.
  • Evidence: Lines 205-214 of src/winapp-npm/src/cli.ts (only the older subcommands). Nested help was updated in the diff but showCombinedHelp() wasn't.
  • Recommendation: Add the new subcommands and an example to showCombinedHelp() so the new flow is discoverable from the main entry point.

M11 src/winapp-VSC/package.json:18-120

  • Severity: medium · Confidence: medium · Domain: docs-and-samples
  • Finding: VS Code extension doesn't surface the new winapp node jsbindings commands in activationEvents / contributes.commands. Either (a) extension parity is missing, or (b) this is intentionally npm-only and should be documented.
  • Evidence: Manifest stops at the existing surface (onCommand:winapp.certInfo, "command": "winapp.certInfo", "title": "Certificate Info"); no jsbindings entries.
  • Recommendation: Add Command Palette entries for the JS bindings flows under "category": "WinApp", or add a short note in the extension README explaining the intended product boundary.

🤖 Generated by the pr-review skill in GitHub Copilot CLI. Orchestrator: Claude Opus 4.7. Specialist dimensions: security, correctness, cli-ux, alternative-solution, test-coverage, docs-and-samples, packaging. Multi-model cross-check: GPT-5.4.

Comment thread docs/guides/electron/jsbindings.md Outdated
Comment thread docs/guides/electron/jsbindings.md Outdated
Comment thread docs/guides/electron/jsbindings.md Outdated
Comment thread docs/fragments/skills/winapp-cli/frameworks.md Outdated
@lei9444
Copy link
Copy Markdown
Contributor Author

lei9444 commented May 19, 2026

addressed all comments
image

Copy link
Copy Markdown
Member

@nmetulev nmetulev left a comment

Choose a reason for hiding this comment

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

  • Should we have winapp init detect when called from npm package and show an option to the user to setup C++, JS, or both bindings? I feel that would simplify the process and remove the need for the jsbindings commands which can be removed - we can simply use winapp init and winapp restore like we do for c++
  • The --js-bindings-ai feels odd, why are we adding a single property just for AI and not for anything else? Could we do without this option?

Comment thread docs/cli-schema.json Outdated
Comment thread docs/cli-schema.json Outdated
Comment thread docs/cli-schema.json Outdated
Comment thread docs/guides/electron/index.md Outdated
Comment thread docs/guides/electron/index.md Outdated
Comment thread docs/guides/electron/jsbindings.md Outdated
Comment thread docs/js-bindings.md Outdated
Comment thread docs/js-bindings.md Outdated
Comment thread src/winapp-CLI/WinApp.Cli/Services/UserPackageJsonService.cs Outdated
Comment thread src/winapp-CLI/WinApp.Cli/Services/DynWinrtCodegenService.cs Outdated
Comment thread src/winapp-CLI/WinApp.Cli/Services/ConfigService.cs Outdated
Comment thread docs/guides/electron/jsbindings.md Outdated
Comment thread src/winapp-CLI/WinApp.Cli/Services/WorkspaceSetupService.Prompts.cs Outdated
@nmetulev

This comment was marked as outdated.

Comment thread .github/plugin/agents/winapp.agent.md Outdated
Comment thread .github/plugin/agents/winapp.agent.md Outdated
Comment thread .github/plugin/agents/winapp.agent.md Outdated
Comment thread .github/plugin/agents/winapp.agent.md Outdated
Comment thread .github/plugin/skills/winapp-cli/frameworks/SKILL.md Outdated
Comment thread .github/plugin/skills/winapp-cli/frameworks/SKILL.md Outdated
Comment thread .github/plugin/skills/winapp-cli/frameworks/SKILL.md Outdated
Comment thread docs/fragments/skills/winapp-cli/frameworks.md Outdated
Comment thread docs/guides/electron/index.md Outdated
Comment thread docs/guides/electron/index.md Outdated
Comment thread docs/guides/electron/index.md Outdated
Comment thread docs/usage.md Outdated
Comment thread docs/usage.md Outdated
Comment thread docs/usage.md Outdated
Comment thread docs/usage.md Outdated
Comment thread docs/usage.md Outdated
Comment thread docs/guides/electron/js-bindings.md Outdated
Copy link
Copy Markdown
Contributor

@chiaramooney chiaramooney left a comment

Choose a reason for hiding this comment

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

@nmetulev just an overall question, are we okay with the init command defaulting to Yes for generating bindings in Electron apps? Or should we default to No?

Comment thread docs/guides/electron/js-bindings.md Outdated
Comment thread docs/guides/electron/js-bindings.md Outdated
Comment thread src/winapp-npm/src/jsbindings/spinner.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants