Repository: twitch-apis/twitch-js
Pull request #113 (2018-07-26 15:15:26)

Bugfix: fallback to `setTimeout` if `setImmediate` is not defined
## Proposed changes

This change could be useful when `twitch-js` is used by another Webpack module which doesn't implement `setImmediate`.

## Types of changes

What types of changes does your code introduce to TwitchJS?
_Put an `x` in the boxes that apply and remove those that do not apply_

* [x] Chore (change that does not fix an issue or add functionality, but improves TwitchJS in some other way)

## Checklist

_Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code._

* [x] I have read the [CONTRIBUTING](https://github.com/twitch-apis/twitch-js/blob/master/CONTRIBUTING.md) doc
* [x] My PR is named according to [CONTRIBUTING](https://github.com/twitch-apis/twitch-js/blob/master/CONTRIBUTING.md) doc
* [x] Lint and unit tests pass locally with my changes
* [x] I have added tests that prove my fix is effective or that my feature works
* [x] I have added necessary documentation (if appropriate)
* [x] Any dependent changes have been merged and published in downstream modules

## Further comments

When implementing the new `twitch-js` library in my browser client, the `setImmediate` error happened, causing various issues. With this fix, either people wanting to build an application from transpiled sources for web, node or other targets can do it without effort.

marcandrews: Thanks for your PR 🎉

I have to admit that I have only been testing the `dist` build of v2 and haven't tested it as a dependency. Webpack is used to build `dist` and there, `setImmediate` **is** shimmed. I will have to test it myself, but until then, can you share your Webpack version/config?

As you can see, we would have to add tests to for this PR to pass; maybe we can shim instead?

diamondio/better-queue#27 has more information.

Let me know what you think.
Seldszar: I totally agree with you, Webpack should shim `setImmediate`.
After you told me to share the Webpack setup, I checked and it was in fact the tool I was using generating the configuration, Vue CLI.
For some reason, `setImmediate` shimming is set to `false`, causing this shitstorm and I don't even know why.
So I think this pull request is now invalid/useless.
marcandrews: It might be good to shim it on our side as well, just in case of special situations like this, or if another bundler is being used, like Browserify.
Seldszar: You got a point, completely forgot about the other bundlers.
Well, if a shim could be the easiest solution, I can do that if you want.
Seldszar: Another solution could be to replace the workaround by `asap` (`setImmediate: cb => asap(cb)`, for example), the coverage doesn't decrease and we keep the purpose of `setImmediate`.
The only downside will be the presence of another dependency.
marcandrews: Closing this in favor of #115.
marcandrews: Resolves #115.

For testing `setIntermediate` and `setTimeout` see how I mocked `Date` [here](https://github.com/twitch-apis/twitch-js/blob/next/src/Chat/__tests__/Client.test.js).

To summarize:
```js
let realSetIntermediate

beforeAll(() => {
  realSetIntermediate = setIntermediate
})

afterAll(() => {
  global.setIntermediate = realSetIntermediate
})

test('should return setTimeout when setIntermediate is undefined', () => {
  global.setIntermediate = undefined

  // ... execute setIntermediate utility test
})

```
Seldszar: Is it not `Bugfix` instead? (according to CONTRIBUTING.md)
marcandrews: Oops you're right. Fixed.

Patch:
diff --git a/src/Chat/Queue.js b/src/Chat/Queue.js
index 64200f7..ae77b3e 100644
--- a/src/Chat/Queue.js
+++ b/src/Chat/Queue.js
@@ -2,6 +2,7 @@ import BetterQueue from 'better-queue'
 import MemoryStore from 'better-queue-memory'
 
 import * as constants from './constants'
+import { defer } from './utils'
 
 class Queue extends BetterQueue {
   rateLimiter = 0
@@ -17,6 +18,7 @@ class Queue extends BetterQueue {
       {
         store: new MemoryStore(),
         priority: ({ priority = 1 }, cb) => cb(null, priority),
+        setImmediate: defer,
         // Process queue only when rate-limiter is less than 1.
         precondition: cb => cb(null, this.rateLimiter < 1),
         preconditionRetryTimeout: constants.QUEUE_TICK_RATE,
diff --git a/src/Chat/utils/index.js b/src/Chat/utils/index.js
index 83dab21..e2b9f47 100644
--- a/src/Chat/utils/index.js
+++ b/src/Chat/utils/index.js
@@ -15,4 +15,12 @@ const getMessageQueueWeight = ({
   return constants.RATE_LIMIT_USER
 }
 
-export { getMessageQueueWeight }
+const defer = cb => {
+  if (typeof self.setImmediate !== 'undefined') {
+    return self.setImmediate(cb)
+  }
+
+  return self.setTimeout(cb, 0)
+}
+
+export { getMessageQueueWeight, defer }


Test patch:
diff --git a/src/Chat/utils/__tests__/index.test.js b/src/Chat/utils/__tests__/index.test.js
new file mode 100644
index 0000000..71fa1a1
--- /dev/null
+++ b/src/Chat/utils/__tests__/index.test.js
@@ -0,0 +1,31 @@
+import * as utils from '../index'
+
+describe('defer', () => {
+  const realSetImmediate = self.setImmediate
+
+  beforeEach(() => {
+    self.setImmediate = realSetImmediate
+  })
+
+  afterAll(() => {
+    self.setImmediate = realSetImmediate
+  })
+
+  test('should execute when setImmediate is defined', async () => {
+    const promise = new Promise(resolve => {
+      utils.defer(resolve)
+    })
+
+    await expect(promise).resolves.toBeUndefined()
+  })
+
+  test('should execute when setImmediate is undefined', async () => {
+    self.setImmediate = undefined
+
+    const promise = new Promise(resolve => {
+      utils.defer(resolve)
+    })
+
+    await expect(promise).resolves.toBeUndefined()
+  })
+})