fix(hooks): make the defineHook surface validate and refuse silent no-ops

Drops the `I` prefix from the new hook types, makes `run` the handler
field with `defineHook({ name, run })` canonical and the positional call
kept as sugar, and validates the definition at define time: a missing or
non-string name, a non-function run, and unknown fields all throw naming
the definition and both accepted forms.

The definition marker moves from a non-enumerable defineProperty to a
plain assignment so a spread-derived definition stays recognizable, and
`isHookDefinition` becomes a type predicate.

Behaviors that used to fail quietly now say so:

- `ctx.wrap()` only ever ran at the `@hook`-decorated before-points and
  was dropped everywhere else. Call sites now declare whether they
  consume middlewares, and `wrap()` throws elsewhere instead.
- a definition whose name disagrees with its hook point is skipped with
  a warning rather than run at a point it was not written for.
- `ctx.abort()` with no message produced `Error(undefined)`; it now
  falls back to a message naming the hook point.
- a definition whose run returns a function warns, since the legacy
  returned-middleware convention does not apply to definitions.
- an array export is rejected naming the file, reserving the form for a
  possible multi-definition module later.

`defineHook<TPayload>` / `HookContext<TPayload>` type the payload, as
`TPayload | undefined` because dispatch-fired hooks carry none, and
executeBeforeHooks is typed with the middleware array it already returns.

Author: edusperoni

lib/common/declarations.d.ts

@@ -824,12 +824,23 @@ interface IAutoCompletionService {
 	isObsoleteAutoCompletionEnabled(): boolean;
 }
 
+interface IHookExecutionOptions {
+	/**
+	 * Set by call sites that fold the returned middlewares around a method (the
+	 * `@hook` decorator). Where nothing consumes them, `ctx.wrap()` rejects
+	 * instead of registering a middleware that would never run.
+	 */
+	consumesMiddlewares?: boolean;
+}
+
 interface IHooksService {
 	hookArgsName: string;
+	/** Resolves with the middlewares hooks registered through `ctx.wrap()`. */
 	executeBeforeHooks(
 		commandName: string,
 		hookArguments?: IDictionary<any>,
-	): Promise<void>;
+		options?: IHookExecutionOptions,
+	): Promise<import("./define-hook").HookMiddleware[]>;
 	executeAfterHooks(
 		commandName: string,
 		hookArguments?: IDictionary<any>,

lib/common/define-hook.ts

@@ -8,6 +8,9 @@
  * `Symbol.for` rather than a module-local symbol: an extension may resolve a
  * duplicated copy of the CLI from its own node_modules, and the running CLI
  * still has to recognize definitions minted by that copy.
+ *
+ * Assigned as a plain enumerable property so that `{ ...definition }` keeps the
+ * marker; symbols stay invisible to Object.keys/for..in/JSON either way.
  */
 export const HOOK_DEFINITION_MARKER = Symbol.for(
 	"nativescript:cli:hookDefinition",
@@ -22,15 +25,20 @@ export type HookMiddleware = (
 	next: (...args: any[]) => any,
 ) => any;
 
-export interface IHookContext {
+export interface HookContext<TPayload = any> {
 	/**
 	 * The payload of the operation being hooked. Its shape depends on the hook
 	 * point, and it is the caller's own object: mutating it is a supported
-	 * channel for influencing the operation.
+	 * channel for influencing the operation. Hook points fired by command
+	 * dispatch carry no payload at all, hence `undefined`.
 	 */
-	payload: any;
+	payload: TPayload | undefined;
 
-	/** Registers a middleware around the method this hook point decorates. */
+	/**
+	 * Registers a middleware around the method this hook point decorates.
+	 * Available only to before-hooks of the hook points that fold middlewares
+	 * around a method; elsewhere it throws rather than dropping the middleware.
+	 */
 	wrap(middleware: HookMiddleware): void;
 
 	/**
@@ -40,52 +48,173 @@ export interface IHookContext {
 	abort(message: string, opts?: { asWarning?: boolean }): never;
 }
 
-export type HookHandler = (ctx: IHookContext) => void | Promise<void>;
+export type HookHandler<TPayload = any> = (
+	ctx: HookContext<TPayload>,
+) => void | Promise<void>;
+
+/** The object bag accepted by `defineHook`. */
+export interface HookDefinitionInput<TPayload = any> {
+	/** Hook point, in the hyphen convention: `before-prepare`, `after-watch`. */
+	name: string;
+	run: HookHandler<TPayload>;
+}
 
-export interface IHookDefinition {
+export interface HookDefinition<TPayload = any> {
 	/** Hook point, in the hyphen convention: `before-prepare`, `after-watch`. */
 	readonly name: string;
-	readonly handler: HookHandler;
+	readonly run: HookHandler<TPayload>;
 }
 
-export interface IHookInvocation {
-	context: IHookContext;
+export interface HookInvocation<TPayload = any> {
+	context: HookContext<TPayload>;
 	/** Populated by `ctx.wrap()` while the handler runs. */
 	middlewares: HookMiddleware[];
 }
 
-export function defineHook(
+const DEFINITION_FIELDS = ["name", "run"];
+
+const ACCEPTED_FORMS =
+	'defineHook({ name: "before-prepare", run: (ctx) => {} }) or ' +
+	'defineHook("before-prepare", (ctx) => {})';
+
+function describeDefinition(name: any): string {
+	return typeof name === "string" && name.length
+		? JSON.stringify(name)
+		: "<unnamed>";
+}
+
+function failToDefine(message: string): never {
+	throw new Error(`${message} Accepted forms: ${ACCEPTED_FORMS}.`);
+}
+
+export function defineHook<TPayload = any>(
+	definition: HookDefinitionInput<TPayload>,
+): HookDefinition<TPayload>;
+export function defineHook<TPayload = any>(
 	name: string,
-	handler: HookHandler,
-): IHookDefinition {
-	const definition: IHookDefinition = { name, handler };
-	Object.defineProperty(definition, HOOK_DEFINITION_MARKER, { value: true });
+	run: HookHandler<TPayload>,
+): HookDefinition<TPayload>;
+export function defineHook<TPayload = any>(
+	nameOrDefinition: string | HookDefinitionInput<TPayload>,
+	run?: HookHandler<TPayload>,
+): HookDefinition<TPayload> {
+	const input = normalizeDefinitionInput(nameOrDefinition, run);
+	const definition: any = { name: input.name, run: input.run };
+	definition[HOOK_DEFINITION_MARKER] = true;
+
 	return definition;
 }
 
-export function isHookDefinition(value: any): boolean {
+function normalizeDefinitionInput<TPayload>(
+	nameOrDefinition: string | HookDefinitionInput<TPayload>,
+	run?: HookHandler<TPayload>,
+): HookDefinitionInput<TPayload> {
+	if (typeof nameOrDefinition === "string") {
+		if (!nameOrDefinition.length) {
+			failToDefine("defineHook() requires a non-empty hook point name.");
+		}
+
+		if (typeof run !== "function") {
+			failToDefine(
+				`defineHook(${describeDefinition(nameOrDefinition)}) requires a handler function as its second argument.`,
+			);
+		}
+
+		return { name: nameOrDefinition, run };
+	}
+
+	if (
+		!nameOrDefinition ||
+		typeof nameOrDefinition !== "object" ||
+		Array.isArray(nameOrDefinition)
+	) {
+		failToDefine("defineHook() was called with an unsupported argument.");
+	}
+
+	const unknownFields = Object.keys(nameOrDefinition).filter(
+		(field) => DEFINITION_FIELDS.indexOf(field) === -1,
+	);
+	if (unknownFields.length) {
+		failToDefine(
+			`defineHook(${describeDefinition(nameOrDefinition.name)}) received unknown ` +
+				`field${unknownFields.length > 1 ? "s" : ""} ` +
+				`${unknownFields.map((field) => JSON.stringify(field)).join(", ")}. ` +
+				`Supported fields: ${DEFINITION_FIELDS.map((field) => JSON.stringify(field)).join(", ")}.`,
+		);
+	}
+
+	if (typeof nameOrDefinition.name !== "string" || !nameOrDefinition.name) {
+		failToDefine(
+			'defineHook() requires a non-empty "name" naming the hook point.',
+		);
+	}
+
+	if (typeof nameOrDefinition.run !== "function") {
+		failToDefine(
+			`defineHook(${describeDefinition(nameOrDefinition.name)}) requires "run" to be a function.`,
+		);
+	}
+
+	return { name: nameOrDefinition.name, run: nameOrDefinition.run };
+}
+
+export function isHookDefinition<TPayload = any>(
+	value: any,
+): value is HookDefinition<TPayload> {
 	return (
 		!!value &&
 		(typeof value === "object" || typeof value === "function") &&
 		value[HOOK_DEFINITION_MARKER] === true &&
-		typeof value.handler === "function"
+		typeof value.run === "function" &&
+		typeof value.name === "string"
 	);
 }
 
+export interface HookInvocationOptions {
+	/** The hook point the definition runs at; used in diagnostics. */
+	hookName: string;
+	/**
+	 * Whether the caller folds the collected middlewares around a method. Only
+	 * the `@hook`-decorated before-points do; everywhere else `ctx.wrap()` has
+	 * nothing to wrap and says so instead of silently dropping the middleware.
+	 */
+	consumesMiddlewares?: boolean;
+}
+
 /**
  * Derives the context from the raw hook argument bag: the `hookArgs` wrapper
  * when the hook point supplies one, the bag itself for hook points that pass
  * their keys at the top level, and nothing when there is no payload.
  */
-export function createHookInvocation(hookArguments: any): IHookInvocation {
+export function createHookInvocation<TPayload = any>(
+	hookArguments: any,
+	options: HookInvocationOptions,
+): HookInvocation<TPayload> {
+	const { hookName, consumesMiddlewares } = options;
 	const middlewares: HookMiddleware[] = [];
-	const context: IHookContext = {
+	const context: HookContext<TPayload> = {
 		payload: derivePayload(hookArguments),
 		wrap(middleware: HookMiddleware): void {
+			if (!consumesMiddlewares) {
+				throw new Error(
+					`ctx.wrap() is not available at the "${hookName}" hook point: nothing folds the middleware around a method there, so it would never run.`,
+				);
+			}
+
+			if (typeof middleware !== "function") {
+				throw new Error(
+					`ctx.wrap() expects a function at the "${hookName}" hook point.`,
+				);
+			}
+
 			middlewares.push(middleware);
 		},
 		abort(message: string, opts?: { asWarning?: boolean }): never {
-			const error: any = new Error(message);
+			const text =
+				typeof message === "string" && message.trim().length
+					? message
+					: `The "${hookName}" hook aborted without a message.`;
+			const error: any = new Error(text);
 			if (opts && opts.asWarning) {
 				// The pair the hooks service checks for to downgrade a rejection.
 				error.stopExecution = false;

lib/common/helpers.ts

@@ -615,6 +615,7 @@ export function hook(commandName: string) {
 			return hooksService.executeBeforeHooks(
 				commandName,
 				prepareArguments(method, args, hooksService),
+				{ consumesMiddlewares: true },
 			);
 		},
 		async (method: any, self: any, resultPromise: any, args: any[]) => {