Base.prototype.isRegistered defaults to register = true

[mootari]

Version: 0.11.2 (the one used by the latest assemble release)

Something I noticed while creating typings for base:
In Base.prototype.isRegistered(name, register), the param register is implicitely documented as defaulting to false. What actually seems to happen though is that the plugin will get registered unless false is passed explicitely. (Source)

[jonschlinkert]

Thanks for catching that. Want to do a pr to fix? Or I can do it as soon as I have a chance.

[mootari]

What part do you want fixed? The code or the annotation?

[mootari]

What part do you want fixed? The code or the annotation?

Bump.

[jonschlinkert]

Sorry, I missed the notif for your reply. Thanks for the bump.

What part do you want fixed? The code or the annotation?

IMHO just the docs should be corrected, but I'm open to code fixes too if you were thinking the behavior isn't what is expected. Thoughts?

[mootari]

I'd opt for a code change that matches the annotated examples. .isRegistered() implies a getter, and modifying the state should be an active choice. As a consequence implementing code should become easier to follow.

Unfortunately there is really no good way to deprecate the old signature. I'd argue though that the annotation is the contract and the current implementation should be considered a bug.

[jonschlinkert]

.isRegistered() implies a getter

Perhaps we're thinking of different uses for the method, but how would that imply a getter, given that it can't ever return a useful value without receiving at least one argument? The .isRegistered() method is supposed to tell you if a plugin with the given name has been registered, that wouldn't work as a getter.

Edit: I still think fixing the docs is the best approach. We use base extensively and every plugin, generator and place where a check is done to see if a plugin is registered would need to be updated if we inverted registered to default to false.

Regarding the docs being a contract, that's a bit like saying a book should be updated to match its cover, isn't it?

[mootari]

Perhaps we're thinking of different uses for the method, but how would that imply a getter, given that it can't ever return a useful value without receiving at least one argument?

Aplogies, I meant "getter" in the sense that the function returns information about a state. In my opinion .is* is a commonly used naming pattern with an implied semantic (isArray(v), isMatch(v), .isGlob(v), isValidInstance(v) etc). I think suddenly having a method that changes the queried state by default is not intuitive.

Regarding the docs being a contract, that's a bit like saying a book should be updated to match its cover, isn't it?

1. If one uses the function like the example shows then what they get out is not the expected/documented behavior. I'd call that a bug.
2. If someone dug into the code, noticed the discrepancy between documentation and actual code, and started passing arguments according to the actual code (without creating a bug report about it, nonetheless) then they're using an undocumented behavior.
3. For anyone calling .isRegistered according to the documentation nothing changes unless they have a bug in their implementation.

I think 3. is the most important part and a strong argument for changing the behavior instead of the docs. If on the other hand you guys are the primary consumers by a large margin and 2. applies to you, then I guess there's nothing to argue about. In that case one has to be pragmatic and update the docs instead.

[mootari]

The base tests test the implicit/internal signature, which is also used by assemble-loader and base-config. Changing the docs it is, I guess.