Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce useRequestIdleCallback optimization hook #209

Open
gremo opened this issue Jul 11, 2022 · 4 comments
Open

Introduce useRequestIdleCallback optimization hook #209

gremo opened this issue Jul 11, 2022 · 4 comments

Comments

@gremo
Copy link

gremo commented Jul 11, 2022

Why not introduce useRequestIdleCallback to use window.requestIdleCallback() easily in controllers?

Queues a function to be called during a browser's idle periods. This enables developers to perform background and low priority work on the main event loop.

Can I use requestIdleCallback

WIP hook example in JavaScript
/**
 * @typedef {import('@hotwired/stimulus').Controller} Controller
 *
 * @typedef {Object} HookOptions
 * @property {number} [maxWait]
 */

const defaultOptions = {
  maxWait: undefined,
};

/**
 * @param {Controller} controller
 * @param {HookOptions} options
 * @returns {[function(): void, function(): void]}
 */
export const useRequestIdleCallback = (controller, options) => {
  const {
    maxWait,
  } = { ...defaultOptions, ...options };

  const idleCallbackIds = [];

  const idling = (fn, timeout) => {
    if (requestIdleCallback) {
      return function (...args) {
        const idleCallbackId = requestIdleCallback(deadline => fn.call(this, ...args, deadline), { timeout });

        idleCallbackIds.push(idleCallbackId);
      }
    }

    const start = Date.now();

    return function (...args) {
      const timeoutId = setTimeout(() => {
        fn.call(this, ...args, { didTimeout: false, timeRemaining: () => Math.max(0, 50 - (Date.now() - start)) });
      }, 1);

      idleCallbackIds.push(timeoutId);
    };
  };

  const cancelIdling = id =>
    (cancelIdleCallback || clearTimeout)(id);

  const observe = () => {
    controller.constructor.idles?.forEach(func => {
      if ('string' === typeof func) {
        controller[func] = idling(controller[func].bind(controller), maxWait);
      }

      if ('object' === typeof func) {
        const { name, maxWait } = { ...func, ...options };
        if (!name) {
          return;
        }

        controller[name] = idling(controller[name].bind(controller), maxWait);
      }
    });
  };

  const unobserve = () => {
    idleCallbackIds.forEach(id => cancelIdling(id));
  };

  const controllerDisconnect = controller.disconnect.bind(controller);

  Object.assign(controller, {
    disconnect() {
      unobserve();
      controllerDisconnect();
    },
  });

  observe();

  return [observe, unobserve];
}

Usage example:

<a data-controller="test"
  data-action="test#foo">
  Invoke using useRequestIdleCallback
</a>
// test_controller.js
import { Controller } from '@hotwired/stimulus';
import { useRequestIdleCallback } from './hooks/useRequestIdleCallback';

export default class extends Controller {
  static idles = ['foo'];

  connect() {
    useRequestIdleCallback(this);
  }

  foo(e, deadline) {
    console.log({ e, deadline });
  }
}
@marcoroth
Copy link
Member

marcoroth commented Jul 25, 2022

Hey @gremo, thanks for opening this! This looks interesting and is also the first time I have heard of this API!

I don't see a reason why we wouldn't/shouldn't introduce this. The only thing I don't "love" is static idles and useRequestIdleCallback() from a naming-perspective. It somehow doesn't seem super intuitive, but I also don't have a concrete better idea so that we still match the original name of the API.

Nevertheless, do you want to whip up a PR for it so we can discuss the details in there?

@gremo
Copy link
Author

gremo commented Jul 25, 2022

Sure, thanks! I'll made it in a few days as soon as I have some spare time 😁

@gremo
Copy link
Author

gremo commented Jul 26, 2022

@marcoroth done, missing tests and some playgrounds. Let me know, i've changed the naming as you requested. WIP PR #212

@marcoroth
Copy link
Member

Thanks, let's continue the discussion in the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants