RubenVerborgh / AsyncIterator

An asynchronous iterator library for advanced object pipelines in JavaScript
https://rubenverborgh.github.io/AsyncIterator/docs/
Other
48 stars 7 forks source link

README: mention necessity of setImmediate shim for browsers #12

Closed JordanShurmer closed 4 years ago

JordanShurmer commented 5 years ago

Overview

This library makes heavy use of setImmediate, which is a non-standard (at apparently never will be standard) api. See the mdn docs: https://developer.mozilla.org/en/docs/Web/API/Window/setImmediate

I feel like this should be addressed, but am also curious why this has not already come up as an issue. Perhaps all downstream users of this just use the setimmediate npm package polyfill that MDN recommends?

Some thoughts on possible solutions:

  1. use the https://www.npmjs.com/package/setimmediate package
  2. or replace setImmediate calls with setTimeout

Use the polyfill

The thing that makes this complicated is that there is no build/bundling process in this repo yet, and this would require one, right?

use setTimeout

This was the first thing that came to mind, so I forked the repo and tested it. A bunch of unit tests failed though... not sure why at this point

RubenVerborgh commented 5 years ago

Perhaps all downstream users of this just use the setimmediate npm package polyfill that MDN recommends?

webpack takes care of this: https://webpack.js.org/configuration/node#nodesetimmediate

not sure why at this point

https://javascript.info/microtask-queue

JordanShurmer commented 5 years ago

webpack takes care of this

Interesting, I hadn't come across this feature of webpack yet.

The problem is I'm trying to use this in a project that does not use webpack. I've added the setimmediate package as a dependency directly and required it, but it'd be nice if this wasn't necessary. I suppose the README could be updated at least for now, to indicate that this is necessary

RubenVerborgh commented 5 years ago

Ha, I just remembered that we actually used to have it: https://github.com/RubenVerborgh/AsyncIterator/commit/37f546033a9440cf45b994d980ef604cec38233d

But the complaint was that this took away people's liberty to use another shim.

I suppose the README could be updated at least for now, to indicate that this is necessary

+1

RubenVerborgh commented 4 years ago

v3.x will use queueMicrotask