hasPermission() implies boolean
I think we should just go with permission() similar to the Notifications API. (The reason it cannot be a property is because we want the result to be asynchronous and the result can change over time.)
Or permissionStatus(). That said, I'm hoping to remove this call from the API and rely on https://w3c.github.io/permissions/ instead.
I think we should remove it. The Push API had a similar surface but it has been removed (indeed in favor of the Permissions API).
https://groups.google.com/a/chromium.org/d/msg/blink-dev/UdGlL9PtBLo/0mPKlCXyYT8J
Pull request coming.
+1. Thanks for fixing it Kenji.
See commit a61f75d3565ae61c94fc5f606b4a00dd5c2ba222
Reopening.
hasPermission() or similar APIs should be part of every API. That they will someday be covered by a permission API doesn't remove the need for them now, and elongates the critical path to shipping this feature usefully, which is an anti-pattern.
I imagined that permissions API would be low risk but you are right. Do I need to send a new pull request or can we just revert it?
On Tue, Feb 24, 2015, 7:01 AM Alex Russell [email protected] wrote:
Reopening.
hasPermission() or similar APIs should be part of every API. That they will someday be covered by a permission API doesn't remove the need for them now, and elongates the critical path to shipping this feature usefully, which is an anti-pattern.
— Reply to this email directly or view it on GitHub https://github.com/slightlyoff/BackgroundSync/issues/39#issuecomment-75646302 .
If we want to return a string we should just name it permission() in line with the Notifications API. If a boolean is fine hasPermission() works.
I've put hasPermission back. I agree that hasPermission sounds like a bool but let's stick with hasPermission since that's what Push has.
I might add that hasPermission for Push is currently not without controversy, so maybe Push is not a good example in this case.
Then I vote for getPermissionStatus() as it's similar to getRegistration()
I wish we made it a little less Java-esque and used shorter names (we frequently get feedback that names are too long). E.g. get(id), getAll(), and permission(), but I guess I'm okay either way...
@annevk : I'm ok with permission() if it allows us to also add it to the Notification API without blocking on the Permissions API. That said, the naming of permission() makes it also look like you can request the permission with the function, which isn't the contract. So I have a mild preference for hasPermission()
Could you explain why you don't think that hasPermission() implies it returns a boolean?
Agree that hasPermission() suggests boolean. permission() or permissionStatus() seem fine to me.
Preference for permissionState() because:
- it doesn't imply a boolean
- it mimics the terminology used in the upcoming Permissions API (PermissionStatus vs. PermissionState)
I prefer getPermission. While I recognize one could mistake it for performing a request from the user, that meaning is not strongly implied, usually get implies a read-only action without such side effects. If it was to request permission from the user, surely it would be called something like requestPermission.
I concede that getPermissionState is more clear, but this alternative seems prohibitively verbose.
I could be ok with permission() or permissionState() but I'd prefer if the method name is a verb.
Can we try to resolve this issue? I'd like the Push API and BackgroundSync API to use the same name, the issue for the Push API is here: https://github.com/w3c/push-api/issues/136
I think insisting on method names being verbs has led to some of the worst, Java-ish APIs on the web platform. More modern APIs have eschewed this restriction, and been much better for it.
permissionState() seems nice.
@domenic : do you have any recent examples where not insisting on verbs worked out well? Are there any articles or posts that pull that discussion together?
Fetch is a great example (.json(), .text(), etc.). DOM is another (.before(), .after(), etc.). ES2015 is another (.entries(), .keys(), .values(), [Symbol.iterator](), etc.). I could give more if you'd like.
@domenic : I think those are good examples, thanks. I'd prefer permissionState() then.
Why not just permission() then, similar to the Notifications API? nudge nudge
@annevk : Wouldn't it be confusing to define something different with the same name? What if the Notifications API wanted to expose a method like this?
If we can ever manage to steer people away from the synchronous way the Notifications API exposes this it's probably because everyone is using the Permissions API and all of this is no longer relevant?
I mostly care about using shorter names where the next is already sufficient.
The notifications API screwed this up by making .permission a synchronous getter. Using it as a pun only invites confusion. This will need a different name. permissionState() or status() seems fine, with status() creating other potential issues.
For the Push API, we went for permissionState() as that seems to have most traction overall: https://github.com/w3c/push-api/issues/136