search

webflow / js-webflow-api

Node.js SDK for the Webflow Data API
https://www.npmjs.com/package/webflow-api
296 stars 95 forks source link

feat: optional token through configuration flag #57

Closed nickfla1 closed 2 years ago

nickfla1 commented 2 years ago

Solves #56

Description

Adds a flag (handleAuthorization) that allows to enable/disable authorization handling by the client, rendering the token property optional if set to false.

johnagan commented 2 years ago

Thanks for the contribution @nickfla1. What's your use-case for this? Is it so you don't have to create a new instance per token? If so, curious why not just make token an optional field instead token?: string and just update the tests accordingly.

nickfla1 commented 2 years ago

Hi @johnagan thanks for your reply.

What's your use-case for this? Is it so you don't have to create a new instance per token?

Most of our API calls, ones directed to Webflow included, go through a Man In The Middle proxy which handles authetication for our clients. So, in our case we don't have the token at all.

If so, curious why not just make token an optional field instead token?: string and just update the tests accordingly.

I didn't want to break current use cases, that might rely on the library's validation in the constructor, but if you say that it won't be a problem, I'd also prefer a cleaner solution.

nickfla1 commented 2 years ago

Hi @johnagan sorry to bother, do you have any feedback on the matter?

johnagan commented 2 years ago

@nickfla1 imo this feels like quite an edge use-case, so I'm not open to changing the interface so much to support it. I'd consider a PR that makes token in the constructor optional, but not so much adding additional arguments.

You may also want to consider just maintaining a fork for your use-case since this SDK doesn't change that often.

nickfla1 commented 2 years ago

Closed in favor of #58