Skip to content

Commit 880f952

Browse files
committed
fix(yok): keep a registered command when a subcommand would shadow it
Registering `a|b` synthesized the `a` dispatcher over whatever already held that name, so a flat `a` registered first became unreachable and the outcome depended on registration order. The dispatcher is now skipped, with a warning naming both commands, unless the record it would replace is one this instance created for the parent itself — either an earlier dispatcher or the placeholder requireCommand puts there so a child's module can be loaded through the parent name. `widget` was the one command relying on the old behavior: its flat registration was dead, clobbered by the dispatcher `widget|ios` synthesized. Dropping it leaves `ns widget` and `ns widget ios` behaving exactly as before.
1 parent e6604a3 commit 880f952

3 files changed

Lines changed: 32 additions & 4 deletions

File tree

‎lib/bootstrap.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,5 +480,5 @@ injector.requireCommand(
480480
],
481481
"./commands/native-add",
482482
);
483-
injector.requireCommand(["widget", "widget|ios"], "./commands/widget");
483+
injector.requireCommand(["widget|ios"], "./commands/widget");
484484
require("./key-commands/bootstrap");

‎lib/commands/widget.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -934,5 +934,6 @@ declare class AppleWidgetUtils extends NSObject {
934934
}
935935
}
936936

937-
injector.registerCommand(["widget"], WidgetCommand);
937+
// No flat "widget": the subcommand registration below synthesizes the parent
938+
// dispatcher, and WidgetCommand serves as WidgetIOSCommand's base class.
938939
injector.registerCommand(["widget|ios"], WidgetIOSCommand);

‎lib/common/yok.ts‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,12 @@ export class Yok extends Injector implements IInjector {
8787
* registered, not because child requires ran out of order.
8888
*/
8989
private synthesizedParents = new Set<string>();
90+
/**
91+
* Parents whose record is only the placeholder requireCommand creates so a
92+
* child's module can be loaded through the parent name. The dispatcher is
93+
* meant to replace it once that module registers itself.
94+
*/
95+
private placeholderParents = new Set<string>();
9096
private KEY_COMMANDS_NAMESPACE: string = "keyCommands";
9197
private hierarchicalCommands: IDictionary<string[]> = {};
9298

@@ -126,6 +132,7 @@ export class Yok extends Injector implements IInjector {
126132
commands.length > 1 &&
127133
!this.has(this.createCommandName(commands[0]))
128134
) {
135+
this.placeholderParents.add(commands[0]);
129136
this.require(this.createCommandName(commands[0]), file);
130137
if (commands[1] && !commandName.match(/\|\*/)) {
131138
this.require(this.createCommandName(commandName), file);
@@ -260,7 +267,7 @@ export class Yok extends Injector implements IInjector {
260267
this.hierarchicalCommands[parentCommandName].push(subCommandName);
261268
}
262269

263-
this.createHierarchicalCommand(parentCommandName);
270+
this.createHierarchicalCommand(parentCommandName, name);
264271
}
265272
});
266273
}
@@ -343,7 +350,27 @@ export class Yok extends Injector implements IInjector {
343350
}
344351
}
345352

346-
private createHierarchicalCommand(name: string) {
353+
private createHierarchicalCommand(name: string, triggeredBy?: string) {
354+
if (
355+
this.has(this.createCommandName(name)) &&
356+
!this.synthesizedParents.has(name) &&
357+
!this.placeholderParents.has(name)
358+
) {
359+
// Overwriting would make the registered command unreachable, which is
360+
// strictly worse than leaving the subcommand unrouted.
361+
const logger = this.get("logger", { optional: true });
362+
if (logger) {
363+
logger.warn(
364+
`'${name}' is already registered as a command of its own, so no ` +
365+
`subcommand dispatcher was created for it${
366+
triggeredBy ? ` and '${triggeredBy}' cannot be reached` : ""
367+
}. Rename one of the two.`,
368+
);
369+
}
370+
371+
return;
372+
}
373+
347374
this.synthesizedParents.add(name);
348375
const factory = () => {
349376
return {

0 commit comments

Comments
 (0)