feat(di): token-based dependency injection behind the Yok facade (phase 1) by edusperoni · Pull Request #6099 · NativeScript/nativescript-cli

edusperoni · 2026-07-29T23:59:54Z

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?

Dependency injection is name-based: annotate() discovers a constructor's dependencies by regex-parsing its source text (fn.toString()), keyed on $-prefixed parameter names. That is why the test suite must run against tsc's emit in dist/ instead of the TypeScript sources (see the comment in vitest.config.ts), why the CLI can never be bundled or minified, and why any emit change that rewrites constructor parameters breaks every registration at runtime with nothing caught at type-check time. The injector (Yok) also fuses four concerns — DI container, lazy module loader, command registry/router, and the public-API builder — and nothing about resolution is typed.

What is the new behavior?

Phase 1 of the DI modernization: a purpose-built, token-based container now backs the injector, while every existing surface keeps working unchanged.

New container (lib/common/di/): Angular-compatible API — inject(), runInInjectionContext(), Injector, provide/provideLazy, forwardRef. Tokens are abstract classes annotated with @Contract({ name }); lookup is class-object first with a per-level fallback to the token name, so string spellings (doctorService, $doctorService), per-call overrides, and duplicated contract copies in the extensions tree all resolve to the same provider. Lazy path-based loading, transient retention, and disposal (reverse instantiation order) are preserved as native provider kinds; a legacy provider kind still constructs unmigrated classes via annotate().
Yok becomes a delegating facade: its entire public surface, subclassability, and global.$injector are intact — storage and resolution moved to the new container. The existing yok unit tests (~67 cases) and the public-API surface test pass unchanged.
nativescript/contracts subpath (via a directory package.json, deliberately no exports map so deep requires keep working) ships the first contract tranche (DoctorService, ProjectNameService); implementations are renamed *Impl, which is externally invisible since outside resolution is by name or token, never class identity.
Deprecation groundwork: legacy APIs now report usage through a central tracer — trace-level today, escalatable to warn/error via a single dial (NS_DEPRECATIONS=warn|error previews) — and the whole legacy surface (Yok members, IInjector, annotate(), @hook) carries @deprecated JSDoc naming its replacement.
Ecosystem contracts pinned by new fixtures: param-name hook signatures (every payload shape and influence channel), require-time extension registration against global.$injector, and the full IInjector facade surface — so future migration steps are gated on third-party behavior, not just internal tests.
@hook() now reaches the global injector as a last resort, so classes migrated off property injection keep their hooks; instance-level $hooksService/$injector still win (test seams preserved).

No behavior change intended: full suite green (1578 tests), no cold-start regression (ns --version ~0.92s vs 0.95s baseline).

Documentation

dependency-injection.md — the new API end to end: @Contract tokens, inject()/runInInjectionContext/Injector, provider kinds (provide, provideLazy, values, factories, shared: false), createInstance with per-call overrides, resolution semantics (class-first/name-fallback, $ normalization, child scopes), forwardRef, coexistence with the legacy $injector (one shared container, NS_DEPRECATIONS staging), guidance for extension/hook authors, the shipped contract tranche, and a legacy → new quick reference.
extending-cli.md — the in-process hooks guide now leads with the typed API: hook bodies run in an injection context (pinned by a compat test), so the recommended pattern is inject(Token) directly in the hook, with hookArgs declared only when the payload is needed; parameter-name injection is demoted to a clearly labeled legacy section. The deprecation tracer matches the guidance: hooks declaring only hookArgs are not flagged — only actual parameter-name service injection is.

API at a glance

import { inject, DoctorService } from "nativescript/contracts";

class PlatformChecker {
	private doctorService = inject(DoctorService); // typed, no decorators needed
}

// declaring a token + registering an implementation
@Contract({ name: "doctorService" })
export abstract class DoctorService { /* abstract members */ }

injector.register(provide(DoctorService, DoctorServiceImpl));
// resolvable as: DoctorService | "doctorService" | "$doctorService" — one singleton

Summary by CodeRabbit

New Features
- Added a token-based dependency-injection API with contracts, provider helpers, forward references, injection contexts, and child scopes.
- Introduced new diagnostics and project-name validation service contracts.
Documentation
- Updated migration and in-process hook documentation to reflect the injection-context model (including legacy behavior notes).
Deprecations
- Added centralized deprecation reporting for legacy hook parameter-name injection and require-time extension registration.
Compatibility
- Preserved legacy extension/command/hook behavior while routing through the new container.
Refactor
- Updated the injector facade, command registry resolution, and service dispatch to use the token-based container.

coderabbitai · 2026-07-30T00:00:01Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

The pull request introduces a typed dependency-injection container, contract-based service tokens, a container-backed legacy Yok facade, configurable deprecation reporting, updated hook guidance, renamed service implementations, compatibility tests, and public contracts package entry points.

Changes

Typed DI migration

Layer / File(s)	Summary
DI contracts and resolution engine `lib/common/di/*`, `test/di.ts`	Adds contract decorators, provider types, scoped injectors, lazy resolution, forward references, lifecycle handling, cycle detection, and token-based resolution tests.
Public contracts and service implementations `lib/contracts/`, `lib/services/`, `contracts/package.json`, `test/contracts.ts`, `test/services/*`, `scripts/copy-assets.js`	Publishes service contracts, renames concrete implementations, updates registrations, copies contract assets, and preserves service behavior.
Yok facade and legacy compatibility `lib/common/yok.ts`, `lib/common/definitions/yok.d.ts`, `test/compat/*`	Routes legacy resolution, registration, commands, lazy loading, enumeration, and disposal through `Injector` while retaining the facade surface.
Deprecation reporting and hook migration `lib/common/deprecation.ts`, `lib/common/helpers.ts`, `lib/common/services/hooks-service.ts`, `lib/services/extensibility-service.ts`, `test/deprecation.ts`, `dependency-injection.md`, `extending-cli.md`	Adds configurable deprecation reporting and documents and tests token-based hooks alongside legacy parameter-name injection.

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

Sequence Diagram(s)

sequenceDiagram
  participant Hook
  participant HooksService
  participant Injector
  participant Service
  Hook->>HooksService: execute in-process hook
  HooksService->>Injector: resolve hook dependencies
  Injector->>Service: get typed or legacy token
  Service-->>Injector: return service
  Injector-->>HooksService: invoke hook in context
  HooksService-->>Hook: return hook result

Poem

A rabbit hops through tokens bright,
With providers queued for day and night.
Old names bow as contracts grow,
Hooks find the services they need to know.
Deprecations softly trace the trail.

🚥 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 accurately summarizes the main change: introducing token-based DI behind the Yok facade as a phased rollout.
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

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

Purpose-built container with an Angular-compatible surface (inject, runInInjectionContext, Injector, provide/provideLazy, forwardRef). Lookup is class-object first with a fallback to the decorator-set name, per injector level, so per-call string overrides and duplicated contract copies in the extensions tree resolve to the same provider. Includes a legacy provider kind that constructs Yok-style classes via annotate(), lazy side-effect loaders for path-based registration, transient retention, and reverse-instantiation-order disposal.

reportDeprecation() dedups per api+detail and logs at trace level by default; NS_DEPRECATIONS=warn|error previews the stricter stages so the same call sites can be escalated over releases. Wired at the external entry points only: param-name hook invocation, require-time extension registration, and dynamicCall help templating.

A class migrated off property injection has neither $hooksService nor $injector, so the decorator threw at hook-execution time. The global injector must stay last in the chain - tests stub the instance properties and rely on them winning.

@deprecated

Yok keeps its entire public surface, subclassability, and the global $injector, while storage and resolution delegate to the new container. Command routing, key commands, and the public-API builder are unchanged. Every legacy member now carries @deprecated JSDoc naming its replacement, mirrored on IInjector.

@contract

DoctorService and ProjectNameService become @contract abstract classes; their impls are renamed *Impl (externally invisible - outside resolution is by string name or token, never class identity). The subpath resolves through contracts/package.json rather than an exports map, so existing deep requires keep working, and the entry point is side-effect-free so a duplicated CLI copy in an extensions tree never boots a second runtime.

Fixtures exercise the surfaces third parties rely on: param-name hook signatures across every payload shape and influence channel (mutation, function-return middleware, abort), require-time extension registration against global.$injector including hierarchical commands, and the full IInjector facade surface. Notably they pin that a hook naming an unwrapped payload key (the after-watchAction shape) is skipped as invalid - resolution changes must not resurrect long-dead hooks.

@contract

dependency-injection.md covers tokens (@contract), inject()/Injector, provider kinds, resolution semantics, forwardRef, coexistence with the legacy $injector, and a legacy-to-new quick reference. extending-cli.md leads with the recommended hook pattern - inject() works directly in hook bodies because they run in an injection context, pinned by a new compat test - and demotes parameter-name injection to a labeled legacy section. String-token lookups are framed as the migration bridge, not a co-equal API. The hook deprecation tracer now flags only hooks that actually use param-name service injection; a hookArgs-only signature follows the recommended pattern and stays silent.

coderabbitai

Actionable comments posted: 4

🧹 Nitpick comments (4)

test/di.ts (1)
1-404: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing coverage for peek(), has(), and remove().

These are public Injector methods with no test exercising them in this file, unlike every other public method (get, register, createChild, createInstance, dispose, getRegisteredNames). Worth adding cases given this is the new core DI surface other layers build on.
🤖 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/di.ts` around lines 1 - 404, Add focused tests in the DI test suite
covering the public Injector methods peek(), has(), and remove(). Verify
presence checks and non-throwing inspection behavior, removal of registered
entries, and the resulting lookup/registration state, including the relevant
child or provider scope behavior exposed by these methods.
lib/common/di/injector.ts (1)
190-225: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

forwardRef isn't resolved for provider value fields, only for provide.

resolveForwardRef is applied to the provide token everywhere (keysFor, findRecordLocal, get), but applyProvider stores useClass/useFactory/useLazyClass verbatim. If a caller wraps an implementation reference in forwardRef (e.g. to defer a TDZ on the impl class itself rather than the token), construct() will do new record.useClass() on the raw forwardRef thunk function and throw a confusing "is not a constructor" instead of the real class. dependency-injection.md only documents forwardRef for the provide position, so this is a latent misuse trap rather than a currently-exercised bug.
♻️ Optional: resolve forwardRef on impl fields too
 		if ("useValue" in provider) {
 			record.kind = "value";
 			record.useValue = provider.useValue;
 			...
 		} else if ("useClass" in provider) {
 			record.kind = "class";
-			record.useClass = provider.useClass;
+			record.useClass = resolveForwardRef(provider.useClass);
 		} else if ("useFactory" in provider) {
 			record.kind = "factory";
 			record.useFactory = provider.useFactory;
 		} else if ("useLazyClass" in provider) {
 			record.kind = "lazyClass";
 			record.useLazyClass = provider.useLazyClass;
 		}
🤖 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/di/injector.ts` around lines 190 - 225, Update applyProvider to
resolve forwardRef-wrapped implementation fields before storing them in
record.useClass, record.useFactory, record.useLazyClass, or
record.useLegacyClass, while preserving existing provider-kind selection and
value handling. Reuse the existing resolveForwardRef helper and leave the
provide-token resolution paths unchanged.
extending-cli.md (1)
124-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use descriptive link text instead of "here".

Flagged by markdownlint (MD059). Screen readers announce link text out of context, so "here" isn't descriptive.
✏️ Suggested fix
-The type of the parameters is described in the `.d.ts` files which are part of the CLI source code [here](https://github.com/NativeScript/nativescript-cli/tree/master/lib/definitions).
+The type of the parameters is described in the [CLI's `.d.ts` definition files](https://github.com/NativeScript/nativescript-cli/tree/master/lib/definitions).
🤖 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 `@extending-cli.md` at line 124, Replace the non-descriptive “here” link text
in the CLI type-definition reference with text that identifies the linked
definitions directory, while preserving the existing destination URL and
surrounding explanation.
Source: Linters/SAST tools
test/compat/legacy-extension.ts (1)
14-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Registered globals aren't cleaned up after the tests.

Both tests register commands/services on the shared global.$injector ("extCompatService", "meowcompat|purr", "meowcompat", "conflictcompat") but never remove them in afterEach. Since this is the process-wide container shared across the whole suite, these registrations persist for the remainder of the test run.
♻️ Suggested cleanup
 	afterEach(() => {
 		fs.rmSync(extDir, { recursive: true, force: true });
 		delete (<any>global).__extCapture;
+		["extCompatService", "commands.meowcompat|purr", "commands.meowcompat", "commands.conflictcompat"].forEach(
+			(name) => (<any>cliGlobal.$injector).di.remove(name),
+		);
 	});
🤖 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/compat/legacy-extension.ts` around lines 14 - 110, Update the afterEach
cleanup in the legacy extension contract tests to remove all registrations added
through the shared global.$injector, including extCompatService,
meowcompat|purr, meowcompat, and conflictcompat, and restore any modified
injector state such as overrideAlreadyRequiredModule. Keep filesystem and global
capture cleanup intact.

🤖 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 `@dependency-injection.md`:
- Around line 120-131: Update the Provider kinds documentation near the useValue
entry to add a one-line caveat that combining useValue with shared: false causes
get() to throw “no resolver registered for ...”, preserving the existing Yok
behavior referenced by the DI tests.

In `@lib/services/doctor-service.ts`:
- Around line 215-217: Update canExecuteLocalBuild to safely handle an omitted
configuration argument before accessing configuration.platform,
configuration.projectDir, or configuration.runtimeVersion. Default the optional
argument to an empty configuration object, or make it required consistently in
the method contract, while preserving the existing getInfos call behavior.
- Around line 31-33: Update the DoctorServiceImpl.printWarnings method to accept
an optional trackResult?: boolean, matching the DoctorService and IDoctorService
contracts while preserving existing behavior when trackResult is provided.
- Around line 181-190: Update the Darwin and Windows branches in
runSetupScript() to return the IS
pawnResult from runSetupScriptCore instead of awaiting and discarding it. Ensure
both platform-specific paths return their core result consistently while
preserving the existing script locations and arguments.

---

Nitpick comments:
In `@extending-cli.md`:
- Line 124: Replace the non-descriptive “here” link text in the CLI
type-definition reference with text that identifies the linked definitions
directory, while preserving the existing destination URL and surrounding
explanation.

In `@lib/common/di/injector.ts`:
- Around line 190-225: Update applyProvider to resolve forwardRef-wrapped
implementation fields before storing them in record.useClass, record.useFactory,
record.useLazyClass, or record.useLegacyClass, while preserving existing
provider-kind selection and value handling. Reuse the existing resolveForwardRef
helper and leave the provide-token resolution paths unchanged.

In `@test/compat/legacy-extension.ts`:
- Around line 14-110: Update the afterEach cleanup in the legacy extension
contract tests to remove all registrations added through the shared
global.$injector, including extCompatService, meowcompat|purr, meowcompat, and
conflictcompat, and restore any modified injector state such as
overrideAlreadyRequiredModule. Keep filesystem and global capture cleanup
intact.

In `@test/di.ts`:
- Around line 1-404: Add focused tests in the DI test suite covering the public
Injector methods peek(), has(), and remove(). Verify presence checks and
non-throwing inspection behavior, removal of registered entries, and the
resulting lookup/registration state, including the relevant child or provider
scope behavior exposed by these methods.

🪄 Autofix (Beta)

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: 7531a01a-933a-4539-98ef-db2c1f39eda9

📥 Commits

Reviewing files that changed from the base of the PR and between 1eae2f6 and 7ff482a.

📒 Files selected for processing (29)

contracts/package.json
dependency-injection.md
extending-cli.md
lib/common/definitions/yok.d.ts
lib/common/deprecation.ts
lib/common/di/contract.ts
lib/common/di/forward-ref.ts
lib/common/di/index.ts
lib/common/di/inject.ts
lib/common/di/injector.ts
lib/common/di/providers.ts
lib/common/helpers.ts
lib/common/services/hooks-service.ts
lib/common/yok.ts
lib/contracts/doctor-service.ts
lib/contracts/index.ts
lib/contracts/project-name-service.ts
lib/services/doctor-service.ts
lib/services/extensibility-service.ts
lib/services/project-name-service.ts
scripts/copy-assets.js
test/compat/injector-facade-surface.ts
test/compat/legacy-extension.ts
test/compat/legacy-hooks.ts
test/contracts.ts
test/deprecation.ts
test/di.ts
test/project-name-service.ts
test/services/doctor-service.ts

coderabbitai

Actionable comments posted: 2

🤖 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 `@extending-cli.md`:
- Around line 80-99: The hook example’s import omits the Injector symbol used by
the late-lookup guidance. Update the shown nativescript/contracts import to
include Injector, and keep the existing inject(Injector) pattern consistent with
that public contracts entry point.

In `@lib/common/deprecation.ts`:
- Around line 33-57: Update reportDeprecation so the deduplication key is added
to reported only after a logger has been resolved and the message has been
delivered via logger.warn or logger.trace. Keep the early return for already
reported keys, but allow retries when no logger is available.

🪄 Autofix (Beta)

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: 81105fc2-45b1-455f-86de-700f67bdb99c

📥 Commits

Reviewing files that changed from the base of the PR and between 7ff482a and a23a901.

📒 Files selected for processing (29)

contracts/package.json
dependency-injection.md
extending-cli.md
lib/common/definitions/yok.d.ts
lib/common/deprecation.ts
lib/common/di/contract.ts
lib/common/di/forward-ref.ts
lib/common/di/index.ts
lib/common/di/inject.ts
lib/common/di/injector.ts
lib/common/di/providers.ts
lib/common/helpers.ts
lib/common/services/hooks-service.ts
lib/common/yok.ts
lib/contracts/doctor-service.ts
lib/contracts/index.ts
lib/contracts/project-name-service.ts
lib/services/doctor-service.ts
lib/services/extensibility-service.ts
lib/services/project-name-service.ts
scripts/copy-assets.js
test/compat/injector-facade-surface.ts
test/compat/legacy-extension.ts
test/compat/legacy-hooks.ts
test/contracts.ts
test/deprecation.ts
test/di.ts
test/project-name-service.ts
test/services/doctor-service.ts

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

lib/common/definitions/yok.d.ts
dependency-injection.md
test/services/doctor-service.ts

createHierarchicalCommand registered the synthesized parent - and its execute path resolved commandsService and errors - through the module-level global injector instead of the instance it was called on, so a parent dispatcher leaked onto the global injector whenever a hierarchical command was registered on any other instance. The require-ordering guard accidentally depended on that leak: with the parent now landing on the same instance, it is exempted for synthesized parents (they exist because a child registered, not because requires ran out of order), and the else branch mirrors the existing default-command skip. Net effect: register-then-require orderings that used to throw 'Default commands should be required before child commands' now work; nothing that worked before changes.

@hook

Internal code no longer reads global.$injector: the two cycle-bound sites (the deprecation tracer's logger fallback and @hook's last-resort lookup) go through a getInjector() accessor required at call time. The global property remains write-only from the CLI's side - it exists as the published surface for extensions and hooks.

The token container was reachable only through a getter on the concrete Yok class, so every crossing from facade-typed code into the new API needed an any cast - the migration's most important seam was invisible to the type system. IInjector now declares readonly di: Injector.

coderabbitai

Actionable comments posted: 1

🤖 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 `@lib/common/yok.ts`:
- Around line 592-601: Update getInjector and the published global.$injector
surface so both always reference the same injector, including when callers
assign global.$injector directly; use a setter or computed alias that updates
the module-local injector rather than a stale cached value. Preserve existing
setGlobalInjector behavior and add a compatibility test covering direct
global.$injector assignment and subsequent getInjector, hook, or legacy
resolution.

🪄 Autofix (Beta)

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: 2fe293f5-439a-47c5-80db-09b2e0b95116

📥 Commits

Reviewing files that changed from the base of the PR and between 55a507e and 9bc9739.

📒 Files selected for processing (5)

lib/common/deprecation.ts
lib/common/helpers.ts
lib/common/yok.ts
test/compat/legacy-hooks.ts
test/deprecation.ts

coderabbitai

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 (1)

lib/common/definitions/yok.d.ts (1)
159-163: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Qualify the $injector migration guidance by injection context.

inject(Injector) is only valid while an Injector is active, so mention that limitation and direct late lookups to an already-obtained Injector via get() instead of instructing callers to call inject(Injector) everywhere.
🤖 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/definitions/yok.d.ts` around lines 159 - 163, Update the
deprecation guidance for $injector to state that inject(Injector) is valid only
within an active Injector context, and direct late lookups to reuse an
already-obtained Injector through get().

🤖 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 `@lib/common/definitions/yok.d.ts`:
- Around line 12-18: Update the exported IInjector compatibility surface to
account for the new readonly di: Injector requirement: either make the bridge
optional or provide it through an appropriate compatibility mechanism for legacy
implementations. Preserve assignability for existing stubs and hook
implementations unless this change is intentionally versioned and documented as
breaking.

---

Outside diff comments:
In `@lib/common/definitions/yok.d.ts`:
- Around line 159-163: Update the deprecation guidance for $injector to state
that inject(Injector) is valid only within an active Injector context, and
direct late lookups to reuse an already-obtained Injector through get().

🪄 Autofix (Beta)

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: 205ebfba-4676-4b3e-8da9-304425e99b24

📥 Commits

Reviewing files that changed from the base of the PR and between 9bc9739 and ec5de90.

📒 Files selected for processing (2)

lib/common/definitions/yok.d.ts
test/compat/legacy-hooks.ts

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

test/compat/legacy-hooks.ts

Yok is now an Injector - class Yok extends Injector - so the new API works on the facade directly (get, register with Providers, createChild, runInInjectionContext($injector, ...)) and inject(Injector) inside legacy-constructed classes returns the facade itself instead of a second, inner container identity. The di bridge is gone. register() dispatches by argument shape: a string first argument is the legacy name-based form, anything else is a Provider. IInjector now extends the Injector class type, which constrains implementers to the real class hierarchy (Yok and its subclasses) - intentional, since the interface only ever described Yok.

@contract

IInjector recomposes from per-subsystem faces - CommandRegistry, KeyCommandRegistry, ModuleRegistry, PublicApiBuilder - each an @contract token the facade registers itself under. One object still implements everything until the subsystems are physically extracted; extraction then becomes a provider swap for the face's token instead of a consumer migration. Consumers can depend on the narrow face they actually use, and deprecation becomes per-face instead of a flat everything-is-legacy. The contracts are internal (lib/common/contracts) and deliberately not re-exported from nativescript/contracts; promoting one is a per-contract decision.

- doctor-service: printWarnings accepts an optional trackResult matching its contracts; runSetupScript returns the setup script result instead of resolving undefined; canExecuteLocalBuild no longer dereferences its optional argument - deprecation tracer: a report dropped for lack of a logger is no longer latched as delivered - it reports once a logger exists - yok: global.$injector is an accessor pair, so a direct third-party assignment stays synchronized with the binding getInjector() reads - docs: note the useValue + shared:false quirk; show where Injector is imported from in the late-lookup example

optional resolves to null instead of throwing for unknown tokens (a found-but-misconfigured provider still throws); skipSelf starts at the parent, escaping a child scope's shadowing entry; self refuses parent fallthrough. self+skipSelf throws. host is deliberately absent - it is an Angular component-tree concept with no analog in this hierarchy. get()'s former second parameter - the legacy per-call ctorArguments bag - had exactly one caller, the facade's own resolve(name, bag). It moves to a protected getWithLegacyArguments channel, so the public get() aligns with Angular's shape today rather than after the legacy paths are deleted.

edusperoni force-pushed the feat/di-modernization-phase1 branch from 28e7169 to 7ff482a Compare July 30, 2026 00:18

edusperoni marked this pull request as ready for review July 30, 2026 00:22

edusperoni marked this pull request as draft July 30, 2026 00:29

edusperoni added 7 commits July 29, 2026 21:30

edusperoni force-pushed the feat/di-modernization-phase1 branch from 7ff482a to a23a901 Compare July 30, 2026 00:31

edusperoni marked this pull request as ready for review July 30, 2026 00:37

coderabbitai Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread dependency-injection.md

Comment thread lib/services/doctor-service.ts

Comment thread lib/services/doctor-service.ts Outdated

Comment thread lib/services/doctor-service.ts

coderabbitai Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread extending-cli.md

Comment thread lib/common/deprecation.ts

edusperoni added 3 commits July 29, 2026 22:33

coderabbitai Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread lib/common/yok.ts

coderabbitai Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread lib/common/definitions/yok.d.ts Outdated

edusperoni added 4 commits July 29, 2026 23:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat(di): token-based dependency injection behind the Yok facade (phase 1) - #6099

feat(di): token-based dependency injection behind the Yok facade (phase 1)#6099
edusperoni wants to merge 14 commits into
mainfrom
feat/di-modernization-phase1

edusperoni commented Jul 29, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 30, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

edusperoni commented Jul 29, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

What is the current behavior?

What is the new behavior?

Documentation

API at a glance

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

edusperoni commented Jul 29, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 30, 2026 •

edited

Loading