registerCommandsIn has a few shortcomings that could be addressed in a backward incompatible major revision, or perhaps in a new call (e.g., registerCommandsInDir) eventually obsoleting registerCommandsIn (hereafter: rCI).
First, and most dangerously, the implementation is much more permissive than the desired semantics, because of require-all. rCI will load all.js and .json files in the whole tree, which is bad day waiting to happen for some poor dev, or a silently compromised server if I can plop a new file into the commands dir which will be loaded next time, unnoticed.
E.g.,
/* cmd1 and cmd2 will be registered under mygrp, but oops and yikes will be silently loaded, too */
.../
oops.json
mygrp/
cmd1.js
cmd2.js
subdir/
yikes.js
Second, the implementation can effectively be called once only — calling it with a different directory later confounds un/loading, because of the single commandsPath member. If rCI should be called only once, or only on the same directory, that should be enforced and documented.
Third, a command's .group attribute ought to be implied by filesystem location. Right now it's possible to put mycmd.js in a parent directory whose name does not match the Command's group, and this again breaks later un/loading. And if it does match, it's redundant. The fact that Commando insists upon command grouping is a good thing! We should take a page from Rails' "convention over configuration" here and demand it. (Commands registered without a file would have to specify their own .group of course.)
Finally, .memberName, if it is still meaningful after addressing the above, should be implied by the command file's basename. Similar reasons to the above — breaks reloading and is redundant work for the dev.
As for implementation, I think the easiest thing to do is to recurse only two levels deep, infer group from subdirectories, and perhaps record the loaded filename at the time of require. If a command object has .loadedFromFile, it can be reloaded. If not, not.
registerCommandsInhas a few shortcomings that could be addressed in a backward incompatible major revision, or perhaps in a new call (e.g.,registerCommandsInDir) eventually obsoletingregisterCommandsIn(hereafter: rCI).First, and most dangerously, the implementation is much more permissive than the desired semantics, because of
require-all.rCIwill load all.jsand.jsonfiles in the whole tree, which is bad day waiting to happen for some poor dev, or a silently compromised server if I can plop a new file into the commands dir which will be loaded next time, unnoticed.E.g.,
Second, the implementation can effectively be called once only — calling it with a different directory later confounds un/loading, because of the single
commandsPathmember. IfrCIshould be called only once, or only on the same directory, that should be enforced and documented.Third, a command's
.groupattribute ought to be implied by filesystem location. Right now it's possible to putmycmd.jsin a parent directory whose name does not match the Command's group, and this again breaks later un/loading. And if it does match, it's redundant. The fact that Commando insists upon command grouping is a good thing! We should take a page from Rails' "convention over configuration" here and demand it. (Commands registered without a file would have to specify their own.groupof course.)Finally,
.memberName, if it is still meaningful after addressing the above, should be implied by the command file's basename. Similar reasons to the above — breaks reloading and is redundant work for the dev.As for implementation, I think the easiest thing to do is to recurse only two levels deep, infer group from subdirectories, and perhaps record the loaded filename at the time of
require. If a command object has.loadedFromFile, it can be reloaded. If not, not.