Remove Callback Usage and Embrace Promise-Based API

[Kyle-Ski]

This PR removes all callback support across node-strava-v3, unifying the library under a promise-based API.

Notably, the _check method in uploads is now a promise-based polling mechanism instead of a callback approach, while still preserving the user-facing statusCallback for progress updates.

Key Changes

API Refactoring:

• Removed done parameters from all endpoints (activities, athlete, clubs, segments, streams, etc.).
• Replaced return done(err) with throw new Error(...) for consistent error handling.

Uploads Module:

• Updated the _checkmethod to be promise-based, eliminating internal callbacks.
• Retained the optional statusCallback for users who want to monitor upload progress.

TypeScript Definitions:

Removed the Callback type and all its references in index.d.ts.

Documentation:

• Updated the README to remove all callback references.
• Included a concise migration guide to assist users in transitioning to the promise-based API.

Impact

• Breaking Change: Existing code that utilizes (args, done) callback style must transition to the new promise-based approach.

• Version Bump: Incremented the version from 2.2.1 to 3.0.0 to reflect the breaking changes in accordance with SemVer guidelines.

Rationale

• Incompatibility: Removing callbacks breaks backward compatibility, necessitating updates in existing implementations using callbacks.

• Clarity for Users: A MAJOR version bump (3.0.0) clearly signals to users the need to review and modify their code to accommodate the changes.

• SemVer Compliance: Aligns with Semantic Versioning guidelines, ensuring version numbers accurately reflect the nature of changes.

Will address #153

[markstos]

Overall, great work. I see there was a good deal of effort here to update all these files and I appreciate that.

My feedback is more about setting up shared style standards to collaborate efficiently going forward.

[markstos]

You are right that .then() and .catch() can be used instead, but I would not recommend them.

My understanding of the history is that these were introduced first, but once async/await were added to the language, it's more natural to use await instead of .then(), and using throw unifies async and sync error handling, which is great.

[markstos]

Now the test suite is mixing callbacks and promises, which is unnecessary because Mocha supports async functions natively.

A clearer style would be with before(async () => {...}). Then, await can be used within the function and it's not necessary to explicitly catch an error only to re-throw it. Also, it will no longer be necessary to call done().

I would go further to use eslint to ban using .then() and .catch() to force ourselves to use await and throw consistently.

[Kyle-Ski]

Can definitely switch this over 👍 For the eslint rule, we can add something like this to our package.json since we already have a "eslintConfig" property there, or would we prefer to make a dedicated .eslintrc.json?

    "rules": {
      "no-restricted-syntax": [
        "error",
        {
          "selector": "CallExpression[callee.property.name='then']",
          "message": "Use async/await instead of .then()."
        },
        {
          "selector": "CallExpression[callee.property.name='catch']",
          "message": "Use async/await instead of .catch()."
        }
      ]
    }

[markstos]

Looks like what we are doing is deprecated in the latest eslint, and we should use eslint.config.{js,mjs,cjs,ts} with the latest version:

https://eslint.org/docs/latest/use/configure/configuration-files#configuration-file

If you could set that up, it would be great.

We don't need to write our own rules, though as there's a eslint-plugin-promise package which as a prefer-await-to-then plugin for this purpose:

https://github.com/eslint-community/eslint-plugin-promise/blob/main/docs/rules/prefer-await-to-then.md

In the eslint.config.js we'll have a diff like this:

--- a/eslint.config.mjs
+++ b/eslint.config.mjs
@@ -3,6 +3,7 @@ import stylistic from '@stylistic/eslint-plugin';
 import tsPlugin from '@typescript-eslint/eslint-plugin'; // Import TypeScript plugin
 import tsParser from '@typescript-eslint/parser'; // Import TypeScript parser
 import nodePlugin from 'eslint-plugin-n';
+import promisePlugin from 'eslint-plugin-promise';
 import globals from 'globals';
 import mustPass from './eslint.must-pass.mjs';
 
@@ -92,6 +93,7 @@ export default [
     },
     plugins: {
       n: nodePlugin,
+      promise: promisePlugin,
       stylistic,
     },
     rules: {
@@ -235,6 +237,7 @@ export default [
       'object-shorthand': 'off',
       'one-var': 'off',
       'operator-assignment': 'off',
+      'promise/prefer-await-to-then': 'error',
       'prefer-arrow-callback': 'off',
       'prefer-const': 'off',
       'prefer-destructuring': [

[Kyle-Ski]

Sounds good! I'll make those changes

[Kyle-Ski]

Little update. Still working through this in my spare time. I was able to update eslint to the most recent and fixed all of the linting errors. When I did, however, most of the tests started to fail in other areas like so:

1) "before all" hook in "activities_test"

  athlete_test
    #get()
{
  name: 'SyntaxError',
  message: "Unexpected 'u'",
  at: 1,
  text: 'undefined'
}
      2) should return detailed athlete information about athlete associated to access_token (level 3)
    #listActivities()
listActivities called with args: { after: 1737063011, before: 1737063011 }
Error in listActivities: {
  name: 'SyntaxError',
  message: "Unexpected 'u'",
  at: 1,
  text: 'undefined'
}

I have a suspicion that the tests have been blocked by linting errors for some time now, but I haven't been able to check. That is what's next on my to do list for this.

[markstos]

Thanks for grinding through this!

[johan1252]

This PR will fix the remaining of #175
However looks like it hasn't been touched in quite some time.

@Kyle-Ski do you still plan to merge this? (after rebase and addressing any remaining issues)
If you don't think you'll have time, happy to take it over to bring it over the line.