drop legacy browsers

[jantimon]

hey :)

would it be possible to drop some legacy browsers and save around 40% bytes?

let messageIds = 0;
const createPromiseWorker = (worker) => {
  let callbacks = {};
  let onMessage = ({ message }) => {
    if (!Array.isArray(message) || message.length < 2) {
      // Ignore - this message is not for us.
      return;
    }
    var [messageId, error, result] = message;
    var callback = callbacks[messageId];

    if (!callback) {
      // Ignore - user might have created multiple PromiseWorkers.
      // This message is not for us.
      return;
    }

    delete callbacks[messageId];
    callback(error, result);
  };

  worker.addEventListener("message", onMessage);

  return (userMessage) =>
    new Promise((resolve, reject) => {
      const messageId = messageIds++;
      callbacks[messageId] = (error, result) =>
        error ? reject(new Error(error.message)) : resolve(result);
      // web worker
      worker.postMessage([messageId, userMessage]);
    });
};

class PromiseWorker {
  constructor(worker) {
    this.p = createPromiseWorker(worker);
  }
  postMessage(message) {
    return this.p(message);
  }
}

this minifies to 324b:

let e=0;class{constructor(s){this.p=(s=>{let r={};return s.addEventListener("message",({message:e})=>{if(Array.isArray(e)&&!(e.length<2)){var[s,t,n]=e,a=r[s];a&&(delete r[s],a(t,n))}}),t=>new Promise((n,a)=>{const o=e++;r[o]=(e,s)=>e?a(new Error(e.message)):n(s),s.postMessage([o,t])})})(s)}postMessage(e){return this.p(e)}}

further more we could export createPromiseWorker in addition to the current class based api
this would reduce the code size when using PromiseWorkers:

const z = e(w);z("hello");

vs the class based api:

const z = new E(w);x.postMessage("hello");

[nolanlawson]

Hm, I sorta feel like it's already plenty small? Bundlephobia says 430B min+gz.

I'm not sure saving ~100 bytes (~0.1kB) is worth dropping legacy browser support...

[jantimon]

it's 40% less before gzip (so 40% less parsing) but I agree that this library is small anyways.. just thought we could further improve it..

would you be open to fix the memory leak?
right now (also with my smaller version) all PromiseWorker instances can't be garbage collected (as it is linked to the worker with the addEventListener)

not sure about the browser support - is it just chrome? because Chrome 51 is now 7 years old

[nolanlawson]

40% of a small number is still small... 🙂 Sorry to be "that guy," but I was curious, so I wanted to put an absolute number on it.

According to a WebPageTest run using an emulated Moto G4 phone, it takes about 8ms of compilation and 10ms of scripting to load this library. So saving 40% on parsing would save about 4ms (7.2ms if we can get 40% of the total). Probably this is due to script compilation no longer being a big bottleneck in Chrome.

It's not really about the browser support; it's more about me needing to update this project, fix all the tests, migrate from Travis to Github Actions... it's an old project and a lot would need updating.

Could you explain more about the memory leak? If the Worker is garbage collected, then the PromiseWorker should be garbage collected too. Browsers typically GC an object's listeners when the object is GC'ed, although I admit I haven't tested web workers so it could be a browser bug.

[jantimon]

It's not really about the browser support; it's more about me needing to update this project, fix all the tests, migrate from Travis to Github Actions... it's an old project and a lot would need updating.

oh I know that pain 😄 - no worries that's why I asked for your thoughts before sending a real pr

40% of a small number is still small...

that's true - I guess there are more suitable cases for optimizations..
I was rather looking for a reason to introduce a function based api 😉

Could you explain more about the memory leak?

You are right - as soon as the worker is garbage collected the PromiseWorker will also be removed.
However there are some reasons to keep the worker around (e.g. expensive initialization & service worker) - in those cases every PromiseWorker instance will stay around

here is a (untested) idea to prevent those memory leaks (expand)

let messageIds = 0;

const createPromiseWorker = (worker, userMessage) => {
  let callback;
  let messageId = messageIds++;
  let onMessage = ({ message }) => {
    if (!Array.isArray(message) || message.length < 2) {
      // Ignore - this message is not for us.
      return;
    }
    var [id, error, result] = message;
    if (id !== messageId) {
      // Ignore - user might have created multiple PromiseWorkers.
      // This message is not for us.
      return;
    }
    worker.removeEventListener("message", onMessage);
    callback(error, result);
  };
  worker.addEventListener("message", onMessage);
  return new Promise((resolve, reject) => {
      callback = (error, result) =>
        error ? reject(new Error(error.message)) : resolve(result);
      // web worker
      worker.postMessage([messageId, userMessage]);
    });
};

class PromiseWorker {
  constructor(worker) {
    this._worker = worker;
  }
  postMessage(message) {
    return createPromiseWorker(worker, message);
  }
}