Support functions for provider.auth and provider.token
#463
Conversation
charlesbjohnson
changed the title
provider.auth and `provider.tokenprovider.auth and provider.token
charlesbjohnson
force-pushed
the
charlesbjohnson/dynamic-endpoints
branch
from
57816a0 to
eadaa03
Compare
| is: 'oauth', | ||
| then: Joi.object({ | ||
| temporary: Joi.string().required(), | ||
| temporary: Joi.alternatives(Joi.string(), Joi.func()).required(), |
There was a problem hiding this comment.
My interest is only in auth and token, but I thought it might be odd to leave this out.
| it('passes through without port', () => { | ||
|
|
||
| expect(OAuth.Client.baseUri('http://example.com/x')).to.equal('http://example.com/x'); | ||
| expect(OAuth.Client.baseUri('https://example.com/x')).to.equal('https://example.com/x'); | ||
| }); |
There was a problem hiding this comment.
This is unrelated, but after my changes we lost coverage on this case. This adds it back.
lib/oauth.js
Outdated
|
|
||
|
|
||
| internals.Client.prototype.temporary = function (oauth_callback) { | ||
| internals.Client.prototype.temporary = function (uri, oauth_callback) { |
There was a problem hiding this comment.
This is a breaking change. The Client is a public interface. Can't change it.
lib/oauth.js
Outdated
|
|
||
|
|
||
| internals.Client.prototype.token = function (oauthToken, oauthVerifier, tokenSecret) { | ||
| internals.Client.prototype.token = function (uri, oauthToken, oauthVerifier, tokenSecret) { |
There was a problem hiding this comment.
Same on every Client endpoint... I thought that was implicit... :-)
There was a problem hiding this comment.
I reverted the changes to these method signatures but kept their ability to resolve endpoints from a function, if a function is provided. It's a bit ugly since it potentially requires setting client.settings.token and client.settings.temporary in two passes, but I think it only applies to those who choose to upgrade to using a function instead of a string.
I didn't catch that Client was exported before (d'oh 😅). I think this should resolve the breaking change, right? Is there something else that I'm missing?
|
Thanks for the changes. I hope to get to it in the next few days. If not, please ping me. |
3 participants
Resolves #461