[bug] imgur.saveClient not working as per docs

[swarajpure]

As per the documentation:

// Setting
imgur.setClientId('aCs53GSs4tga0ikp');

// Getting
imgur.getClientId();

// Saving to disk. Returns a promise.
// NOTE: path is optional. Defaults to ~/.imgur
imgur
  .saveClientId(path)
  .then(() => {
    console.log('Saved.');
  })
  .catch((err) => {
    console.log(err.message);
  });

If we continue just like this, we get the error:

Error: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received undefined

Reason:

• imgur.saveClientId function expects two arguments: clientId and path and we're passing only one that too wrong one. Why wrong? clientId is supposed to be the first argument and then path

What's the fix?

Short:

• Edit the function call: saveClientId('aCs53GSs4tga0ikp', null)

Long:

• Edit the function arguments of imgur.saveClientId to be an object and pass path as null
  Something like:

imgur.saveClientId = async ({ clientId, path = null }) => {
......

• Then edit the function call to be saveClientId({ clientId: 'aCs53GSs4tga0ikp' })

I was struggling for some time with this issue and then finally after looking the library functions could I find the problem. Would be better if we can fix this. I know people can use the v2 but there will be people who would be using v1 as it's stable.

Thank you! 😄

[swarajpure]

Once I get your approval @kaimallea I can work on this issue and send a PR :)
Also, please let me know which way should we follow, the short/long fix?

[kaimallea]

@swarajpure hey, thanks for the thorough issue! All PRs are welcome, would love for you to send in a PR fixing this. saveClientId would definitely work better if it saved a client id that was previously set with setClientId()

Out of curiousity, would it be easier for you if you passed in the client id from an environment variable, rather than saving it to disk? e.g., imgur.setClientId(process.env.IMGUR_CLIENTID);`. Also feel free to ignore my suggestion and send a PR. :)

[swarajpure]

@swarajpure hey, thanks for the thorough issue! All PRs are welcome, would love for you to send in a PR fixing this. saveClientId would definitely work better if it saved a client id that was previously set with setClientId()

Thanks! Please let me know whether I should implement the long fix or the short fix. Modifying the library function might cause tests to break (if any). Just wanted to confirm once before I send in a PR 😉

Out of curiousity, would it be easier for you if you passed in the client id from an environment variable, rather than saving it to disk? e.g., imgur.setClientId(process.env.IMGUR_CLIENTID);`. Also feel free to ignore my suggestion and send a PR. :)

Yes, when I was trying it, I was loading the clientId from the environment variables only 🙂
I just copied the clientId from the README.md, just to maintain the way function calls are there in the README.md

[swarajpure]

Oh, I see there is no test for saveClientId function.

I guess then modifying the library function saveClientId to take an object as parameters would be better (check the long fix in description). Let me know what you think 😄

[kaimallea]

@swarajpure since v1.x is in maintenance/support mode, I'd prefer to limit the change to a fix/patch rather than a new feature or breaking change that would require a minor or major version bump

[kaimallea]

🎉 This issue has been resolved in version 1.0.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀