sagemathinc / cocalc

CoCalc: Collaborative Calculation in the Cloud
https://CoCalc.com
Other
1.17k stars 217 forks source link

deprecation: remove use of the async (https://www.npmjs.com/package/async) library from cocalc #6378

Open williamstein opened 1 year ago

williamstein commented 1 year ago

Back in the dark days of CoffeeScript + CallbackHell, we made very extensive use of the async library (https://www.npmjs.com/package/async) in CoCalc. Most of that code has been rewritten in Typescript using async/await, which is just a vastly superior approach. The point of this ticket is to keep track of and make it clear that we want to rewrite most everything that remains.

MOTIVATION:

I think this is used in about 100 places still:

~/cocalc/src/packages$ git grep async\\.series|wc -l
      85
~/cocalc/src/packages$ git grep async\\.parallel|wc -l
      12

Almost all use in cocalc is just async.series.

Here's the package situation, which is particularly disturbing since there are multiple versions, and none is modern:

~/cocalc/src/packages$ git grep '"async"'|grep package.json
database/package.json:    "async": "^1.5.2",
frontend/package.json:    "async": "^2.6.3",
hub/package.json:    "async": "^1.5.2",
project/package.json:    "async": "^1.5.2",
sync/package.json:    "async": "^1.5.2",
util/package.json:    "async": "^1.5.2",

END STATE: There could be some justifiable use case somewhere for something from the async library, just like callbacks are sometimes useful. This should use the current fully updated version of async.

williamstein commented 1 month ago

I hate callbacks. I can't wait until all of course code is 100% async/await, unless some weird edge case. I just spent an hour or more tracking down this bug in welcome_email:

export function welcome_email(opts): void {
  let body, category, subject;
  opts = defaults(opts, {
    to: required,
    token: required, // the email verification token
    only_verify: false, // TODO only send the verification token, for now this is good enough
    settings: required,
    cb: undefined,
  });

  if (opts.to == null) {
    // users can sign up without an email address. ignore this.
    typeof opts.cb === "function" ? opts.cb(undefined) : undefined;
    return;
  }

  const settings = opts.settings;
  const site_name = fallback(settings.site_name, SITE_NAME);
  const dns = fallback(settings.dns, DNS);
  const url = `https://${dns}`;
  const token_query = encodeURI(
    `email=${encodeURIComponent(opts.to)}&token=${opts.token}`,
  );
  const endpoint = os_path.join(base_path, "auth", "verify");
  const token_url = `${url}${endpoint}?${token_query}`;
  const verify_emails = opts.settings.verify_emails ?? true;

  if (opts.only_verify) {
    // only send the verification email, if settings.verify_emails is true
    if (!verify_emails) return;
...

Note that return with no call to the callback. It wouldn't happen with async/await.