feat(extensions): nativescript.commands map — per-command lazy loading for extensions

[edusperoni]

Based on main — #6099 (DI foundation), #6100 (defineHook) and #6101 (defineCommand) are all merged.

PR Checklist

• The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/tools/notes/CONTRIBUTING.md#commit-messages.
• There is an issue for the bug/feature this PR is for. To avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it.
• You have signed the CLA.
• All existing tests are passing: https://github.com/NativeScript/nativescript-cli/blob/master/CONTRIBUTING.md#contribute-to-the-code-base
• Tests for the changes are included.

What is the current behavior?

Every installed extension is eagerly require()d on every CLI invocation, before the command is even known — the extension's whole module tree loads so its top-level side effects can register commands against global.$injector. nativescript.commands in an extension's package.json is a string[] used only to suggest installs for unknown commands. Two extensions claiming the same command name crash at startup.

What is the new behavior?

nativescript.commands also accepts a map of command name → module path, which becomes authoritative:

"nativescript": {
	"commands": {
		"widget|add": "./dist/commands/widget-add.js",
		"widget|new": { "path": "./dist/commands/widget-add.js" }  // alias: same module, second entry
	}
}

• Per-command lazy loading: nothing from the extension loads until one of its commands actually executes; the extension main is never required, and map-manifest extensions are not flagged by the deprecation tracer (the legacy array/eager path keeps working verbatim, tracer included). Values accept string | { path } so the envelope can grow additively.
• The manifest key is authoritative: routing works before any module loads. If a loaded defineCommand definition's name disagrees with its manifest key, the CLI warns naming both and runs under the key. Aliases are duplicate manifest entries pointing at the same module.
• registerDeferredCommand on the CommandRegistry facet: claiming a name and loading its implementation are now separate registry operations. The registry builds the command record, the parent's subcommand list, and the parent dispatcher from the name alone — a sibling's dispatch never drags in the first claimant's module — and returns a structured DeferredCommandResult (claimed / built-in / subcommand-parent / invalid-name) instead of exception text callers must match on. This is what keeps the future registry extraction a provider swap.
• Failure UX: loader failures name the command, the owning extension, and the module path; a loader that runs but registers nothing fails the same way; non-lowercase manifest keys (permanently unreachable — dispatch lower-cases input) warn and are skipped without sinking the extension's other commands. Built-in conflicts say "already provided by the CLI", no internals.
• Deterministic defaults: *default entries sort first per parent in code — JSON key order carries no meaning. First-wins conflict resolution is defined in the docs (extension load order, alphabetical; the mid-load ns extension install exception documented). Re-declaring a command under the same owner is a no-op, so ns extension install <already-installed> no longer warns about conflicting with itself. "commands": {} opts out of loading entirely.
• A command module may self-register on load (legacy shape) or simply export a defineCommand definition — one registration code path (registerDefinitionAs) serves both the manifest loader and registerCommandDefinition.
• ILazyRequireProvider is no longer part of the exported Provider union (container-internal).
• New authoring guide: extensions.md — leads with the peerDependency + devDependency on nativescript and inject() from nativescript/contracts.

Public type names follow the new-API convention (no I prefix): DeferredCommandOptions, DeferredCommandResult, DeferredCommandRejection.

25 tests in test/extension-manifests.ts (lazy registration, eager-path preservation, malformed/conflict/self-conflict handling, both suggestion shapes, pure-definition modules incl. resolving the parent dispatcher before any child module has loaded, key-mismatch warning, alias entries, {} opt-out). Full stacked suite: 116 files, 1784 passed / 9 skipped; yok oracle, public-API test, and compat fixtures untouched.

Summary by CodeRabbit

• New Features

  • Extensions can declare CLI commands directly in their manifests.
  • Commands load on demand, with support for command definitions and aliases.
  • Extension metadata now reports declared commands.
  • Legacy command manifest formats remain supported.
  • Added validation and clear handling for naming conflicts, malformed entries, and loading failures.
  • Command registration reports clear outcomes when commands cannot be registered.

• Documentation

  • Added comprehensive guidance for creating CLI extensions and configuring commands.
  • Added related-guide links and updated command declaration guidance.

[coderabbitai]

📝 Walkthrough

Walkthrough

The CLI adds deferred command registration for extension manifest maps. It validates command names, resolves ownership conflicts, loads modules lazily, adapts exported command definitions, preserves legacy arrays, and documents supported extension formats.

Changes

Declarative Extension Commands

Layer / File(s)	Summary
Command contracts and container providers lib/common/contracts/..., lib/common/di/..., lib/common/definitions/extensibility.d.ts	Adds deferred command contracts, internal provider types, resolver detection, and extension command metadata.
Deferred command registry lib/common/yok.ts	Adds name validation, ownership tracking, conflict handling, lazy loading, structured failures, and hierarchical command wiring.
Manifest command loading lib/common/services/command-definition-adapter.ts, lib/services/extensibility-service.ts	Registers command maps lazily, supports command definitions and legacy self-registration, preserves array manifests, and reports name mismatches.
Validation and extension documentation test/extension-manifests.ts, extensions.md, defining-commands.md, dependency-injection.md	Adds manifest and loading tests and documents installation, command formats, conflicts, aliases, defaults, and command help configuration.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ExtensionManifest
  participant ExtensibilityService
  participant CommandRegistry
  participant LazyCommandModule
  participant CommandDefinitionAdapter
  ExtensionManifest->>ExtensibilityService: Declare nativescript.commands map
  ExtensibilityService->>CommandRegistry: Register deferred command
  CommandRegistry->>LazyCommandModule: Load command module on lookup
  LazyCommandModule->>CommandDefinitionAdapter: Adapt exported command definition
  CommandDefinitionAdapter->>CommandRegistry: Register definition under manifest name

Loading

Possibly related PRs

• NativeScript/nativescript-cli#6099: Adds DI and Yok registration mechanics used by deferred command registration.
• NativeScript/nativescript-cli#6101: Adds the declarative command system extended by manifest-based command definitions.

Suggested reviewers: nathanwalker

Poem

I’m a rabbit with commands in a map,
Loading each burrow only when tapped.
Conflicts are checked, aliases align,
Definitions register under each sign.
Lazy hops keep the CLI bright—
Tests guard every route just right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely identifies the main change: per-command lazy loading through the nativescript.commands map for extensions.
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.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[NathanWalker]

@copilot resolve the merge conflicts in this pull request

[Copilot]

@copilot resolve the merge conflicts in this pull request

Resolved and pushed in commit 6568c06 by merging origin/main into this branch and fixing the conflicts in defining-commands.md, lib/common/services/command-definition-adapter.ts, and lib/contracts/index.ts.

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (1)

test/extension-manifests.ts (1)

592-619: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the single module load in the alias test.

The test name states that the shared module is loaded once. The body does not assert that. Add an assertion on capture.loadedModules after both resolutions, so a regression that reloads the module per alias fails this test.

♻️ Proposed assertion

 			assert.isOk(testInjector.resolveCommand("nsmalias|run"));
 			const aliased = testInjector.resolveCommand("nsmalias|r");
 			assert.isOk(aliased);
+			assert.deepEqual(capture.loadedModules, ["alias-run"]);
 
 			await aliased.execute(["x"]);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extension-manifests.ts` around lines 592 - 619, Add an assertion to the
alias test after resolving both commands in “routes two aliases of one command
to the same module” that verifies capture.loadedModules contains exactly one
load of the shared module; keep the existing command execution and
capture.executed assertions unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@extensions.md`:
- Line 18: Convert the section headings in extensions.md, including the heading
near “Depending on the CLI” and those at the referenced locations, from ATX
syntax to setext syntax consistent with the file and related documentation. Add
the text language identifier to the unlabeled fenced block near line 263 while
preserving its contents.

In `@lib/common/yok.ts`:
- Around line 159-233: Update registerDeferredCommand to reject a hierarchical
child when its direct parent command is already registered, before calling
super.register or mutating ownership/command state. Add a dedicated structured
reason to DeferredCommandRejection and handle it in describeRejection,
preserving existing behavior for valid parent-child registrations.
- Around line 107-108: Initialize deferredCommandOwners with a null-prototype
object via Object.create(null) instead of a normal object, so command names such
as constructor cannot resolve inherited Object.prototype properties during
ownership checks.

In `@test/extension-manifests.ts`:
- Around line 462-479: Update the fixture generation logic around
definitionModule to resolve the contracts module through Vitest’s requireService
loader before constructing the generated JavaScript, instead of using plain
require.resolve. Ensure the generated fixture embeds the loader-resolved path so
its later plain require can load lib/contracts consistently.

---

Nitpick comments:
In `@test/extension-manifests.ts`:
- Around line 592-619: Add an assertion to the alias test after resolving both
commands in “routes two aliases of one command to the same module” that verifies
capture.loadedModules contains exactly one load of the shared module; keep the
existing command execution and capture.executed assertions unchanged.

🪄 Autofix

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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7b8aebe3-5ac4-44bf-a4d4-08d2aba24c24

📥 Commits

Reviewing files that changed from the base of the PR and between 2f81d7f and 6568c06.

📒 Files selected for processing (13)

• defining-commands.md
• dependency-injection.md
• extensions.md
• lib/common/contracts/command-registry.ts
• lib/common/contracts/index.ts
• lib/common/definitions/extensibility.d.ts
• lib/common/di/index.ts
• lib/common/di/injector.ts
• lib/common/di/providers.ts
• lib/common/services/command-definition-adapter.ts
• lib/common/yok.ts
• lib/services/extensibility-service.ts
• test/extension-manifests.ts

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Inspect how `lib/contracts` resolves and how tests are executed.
set -euo pipefail

fd -a 'contracts' lib --max-depth 2
fd -a 'index.ts' lib/contracts 2>/dev/null || true

# Test runner configuration and TS handling
fd -H -t f 'vitest.config.*|vite.config.*|tsconfig*.json' . --max-depth 2 --exec cat -n {}

# How the test script is invoked
rg -n '"(test|pretest|build)"\s*:' package.json -A2

Repository: NativeScript/nativescript-cli

Length of output: 2626

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate extension-manifests.ts and inspect the relevant fixture-generation snippet plus any build hooks.
fd -a 'extension-manifests.ts' test scripts --max-depth 3 --exec sh -c '
  echo "FILE: $1"
  wc -l "$1"
  sed -n "420,530p" "$1" | cat -n
' sh {}

# Check whether lib/contracts has a package.json or otherwise supports Node require from lib-relative paths.
if [ -f lib/contracts/package.json ]; then
  cat -n lib/contracts/package.json
else
  echo "NO_PACKAGE_JSON"
fi

# Show lockfile/package versions related to vitest/vite/ts to infer runtime support if needed.
for f in package-lock.json pnpm-lock.yaml yarn.lock bun.lock; do
  if [ -f "$f" ]; then
    echo "LOCKFILE: $f"
    rg -n "vitest|typescript|ts-node" "$f" -A2 -B2 | head -n 80 || true
  fi
done

# Behavioral probe from repository file contents using Node without running repository scripts:
# - Resolve the same path from the repo root using node's internal require.resolve semantics if available.
# - Check whether the target file exists at the TypeScript source vs package.json "main" vs compiled dist target.
node - <<'JS'
const fs = require('fs');
const path = require('path');

console.log('NODE_VERSION', process.version);
console.log('CWD', process.cwd());

const source = path.join(process.cwd(), 'lib/contracts/index.ts');
const dist = path.join(process.cwd(), 'dist/lib/contracts/index.js');
const pkg = path.join(process.cwd(), 'lib/contracts/package.json');

console.log('SOURCE_EXISTS', fs.existsSync(source));
console.log('SOURCE_REALPATH', fs.realpathSync(source));
console.log('DIST_EXISTS', fs.existsSync(dist));
console.log('PKG_EXISTS', fs.existsSync(pkg));
console.log('PKG_CONTENT', fs.existsSync(pkg) ? JSON.parse(fs.readFileSync(pkg, 'utf8')) : null);

try {
  const r = require.resolve('../lib/contracts');
  console.log('REQUIRE_RESOLVE', r);
  console.log('REQUIRE_RESOLVE_EXISTS', fs.existsSync(r) || fs.existsSync(r + '.js') || fs.existsSync(r + '.' + (path.extname(r) || '').slice(1)));
  console.log('FS_STAT', fs.statSync(r));
} catch (e) {
  console.log('REQUIRE_RESOLVE_ERROR', e.code || e.message);
}
JS

Repository: NativeScript/nativescript-cli

Length of output: 7774

---

Route requireService through Vitest’s module loader before using require.resolve.

require.resolve("../lib/contracts") resolves inside the test process, but the generated fixture is a compiled .js file that later runs with plain require. Since lib/contracts/index.ts is not a Node module, this can fail unless Vitest’s loader intercepts that load. Make the fixture generation path consistent with test execution.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extension-manifests.ts` around lines 462 - 479, Update the fixture
generation logic around definitionModule to resolve the contracts module through
Vitest’s requireService loader before constructing the generated JavaScript,
instead of using plain require.resolve. Ensure the generated fixture embeds the
loader-resolved path so its later plain require can load lib/contracts
consistently.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[coderabbitai]

🧹 Nitpick comments (1)

test/extension-manifests.ts (1)

611-618: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that the aliased module loads once.

The test name states that two aliases route to the same module. The assertions only confirm that both names resolve. extensions.md documents that the module "is loaded once", and capture.loadedModules can prove it.

♻️ Proposed assertion

 			assert.isOk(testInjector.resolveCommand("nsmalias|run"));
 			const aliased = testInjector.resolveCommand("nsmalias|r");
 			assert.isOk(aliased);
+			assert.deepEqual(capture.loadedModules, ["alias-run"]);
 
 			await aliased.execute(["x"]);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extension-manifests.ts` around lines 611 - 618, Update the test covering
two aliases routing to the same module to assert that capture.loadedModules
contains the aliased module exactly once after executing the alias. Keep the
existing resolution and execution assertions intact, and use the existing
capture.loadedModules value for the assertion.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/extension-manifests.ts`:
- Around line 611-618: Update the test covering two aliases routing to the same
module to assert that capture.loadedModules contains the aliased module exactly
once after executing the alias. Keep the existing resolution and execution
assertions intact, and use the existing capture.loadedModules value for the
assertion.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 144e4ff8-9595-4eaa-b5ab-7a839b0f6f48

📥 Commits

Reviewing files that changed from the base of the PR and between 2f81d7f and da7147b.

📒 Files selected for processing (13)

• defining-commands.md
• dependency-injection.md
• extensions.md
• lib/common/contracts/command-registry.ts
• lib/common/contracts/index.ts
• lib/common/definitions/extensibility.d.ts
• lib/common/di/index.ts
• lib/common/di/injector.ts
• lib/common/di/providers.ts
• lib/common/services/command-definition-adapter.ts
• lib/common/yok.ts
• lib/services/extensibility-service.ts
• test/extension-manifests.ts

🚧 Files skipped from review as they are similar to previous changes (10)

• defining-commands.md
• lib/common/di/index.ts
• lib/common/services/command-definition-adapter.ts
• dependency-injection.md
• lib/common/definitions/extensibility.d.ts
• lib/common/contracts/index.ts
• lib/common/di/providers.ts
• lib/common/contracts/command-registry.ts
• lib/common/yok.ts
• lib/common/di/injector.ts

[edusperoni]

Thanks — went through the review. Three of the four are addressed in 8230926 and 945528a:

• Null-prototype maps — valid, and it applied to hierarchicalCommands too, which was the worse of the two: a manifest key like constructor|foo skipped array initialisation on the truthy inherited value and then threw TypeError: ... push is not a function. Both maps now start from Object.create(null). Audited the rest of yok.ts and extensibility-service.ts: the remaining name-keyed structures are Maps, Sets, or read only through own-key access, so nothing else was reachable.
• Child of an existing parent command — valid, and it broke the point of the structured result: the child record was registered and { registered: true } returned, while createHierarchicalCommand then declined to install a dispatcher, leaving the command permanently unreachable and its ownership recorded. It is now rejected up front, before any state is touched, with a new parent-is-command reason carrying the parent name, so the caller reports a conflict naming the extension, the command and the parent.
• Heading style / fence language — valid, extensions.md was the odd file out; converted to setext to match defining-commands.md and dependency-injection.md, and the install-suggestion block is now labelled.

Not taking the fourth (test/extension-manifests.ts resolving lib/contracts through the Vitest loader): the suite runs the compiled output — vitest.config.ts includes dist/test/**/*.js — so from dist/test/ that require.resolve already lands on dist/lib/contracts, and the generated fixture's plain require() of the absolute path is what we want. Those tests pass as written.

Three tests were added for the fixed behaviours, each verified to fail against the unfixed code first. Suite: 1815 passed / 9 skipped.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

lib/common/yok.ts (1)

211-215: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve diagnostics for non-Error throws.

Line 214 reads err.message. A command module can throw null, undefined, or a string. In that case, this handler throws a new TypeError or loses the original detail. The error then omits the command and source diagnostic.

Proposed fix

 				} catch (err) {
+					const detail = err instanceof Error ? err.message : String(err);
 					throw new Error(
 						`Unable to load command '${name}' of ${options.owner} from ` +
-							`${options.source}: ${err.message}`,
+							`${options.source}: ${detail}`,
 					);
 				}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/common/yok.ts` around lines 211 - 215, Update the catch handler in the
command-loading flow around the visible `catch (err)` block to safely format any
thrown value, including null, undefined, and strings, instead of accessing
`err.message` directly. Preserve the existing command name, owner, and source
context while retaining the original thrown value’s diagnostic detail in the new
error.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@lib/common/yok.ts`:
- Around line 211-215: Update the catch handler in the command-loading flow
around the visible `catch (err)` block to safely format any thrown value,
including null, undefined, and strings, instead of accessing `err.message`
directly. Preserve the existing command name, owner, and source context while
retaining the original thrown value’s diagnostic detail in the new error.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fc9103b2-5dcc-4551-950c-a14e206dadfc

📥 Commits

Reviewing files that changed from the base of the PR and between da7147b and 945528a.

📒 Files selected for processing (5)

• extensions.md
• lib/common/contracts/command-registry.ts
• lib/common/yok.ts
• lib/services/extensibility-service.ts
• test/extension-manifests.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• lib/common/contracts/command-registry.ts
• extensions.md
• lib/services/extensibility-service.ts