Execute queries in Fibers to support yielding APIs. #92
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I have to say: importing non-TypeScript npm packages into TypeScript is a huge pain. This PR took me 1.5 hours instead of ten minutes because of that nonsense. I don't know what I would have done without Visual Studio Code to ease some of the pain. |
| @@ -32,7 +32,19 @@ export interface QueryOptions { | |||
| formatResponse?: Function; | |||
| } | |||
|
|
|||
| // Make the global Promise constructor Fiber-aware. | |||
There was a problem hiding this comment.
Is it possible to move these four lines either to the meteor-integration package or Meteor itself?
The Promise used by Meteor will already be Fiber-enabled, so calling makeCompatible in the tests is just simulating running in a Meteor environment.
Closed
|
Awesome! Really glad that Fibers didn't have to become a dependency for this to work. |
abernix
added a commit
that referenced
this pull request
Jul 3, 2019
…ely. While originally introduced in #92 to accommodate Meteor, it seems that we're no longer making `Promise`s Fiber-aware using the `meteor-promise` library which leverages `fibers` under-the-hood. Since this package seems to be the sole reason that bootstrapping is failing on Node.js 12. To support Node.js 12, we could update to `fibers@4` which accommodates changes to the V8 API which have been made in recent Node.js versions, the `fibers@4` implementation is only compatible with Node.js 10 and 12 and is incompatible with Node.js 8, which we still actively support (though not for much longer!).
abernix
added a commit
that referenced
this pull request
Jul 3, 2019
* Start running tests on Node.js v12. While not currently a so-called "Long-Term-Support" (LTS) release, Node.js v12 is on target to become LTS in October 2019 and Node.js 8 will end LTS earlier than previous even-numbered Node.js releases. See the Node.js schedule: https://github.com/nodejs/Release#release-schedule With that only a few months away, it's time to start getting a read on whether we're going to pass tests in that new engine. * Remove `meteor-promise` and `fibers`, which appear to be unused entirely. While originally introduced in #92 to accommodate Meteor, it seems that we're no longer making `Promise`s Fiber-aware using the `meteor-promise` library which leverages `fibers` under-the-hood. Since this package seems to be the sole reason that bootstrapping is failing on Node.js 12. To support Node.js 12, we could update to `fibers@4` which accommodates changes to the V8 API which have been made in recent Node.js versions, the `fibers@4` implementation is only compatible with Node.js 10 and 12 and is incompatible with Node.js 8, which we still actively support (though not for much longer!).
trevor-scheer
pushed a commit
that referenced
this pull request
May 6, 2020
trevor-scheer
pushed a commit
that referenced
this pull request
May 14, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change makes the global
Promiseconstructor run all its.thencallbacks usingFibers drawn from a pool of recyclableFiberobjects. In other words, any code that runs during query evaluation can now call anyFiber-based Meteor API.I removed the
"no-var-requires": true,lint rule becauseimport Fiber = require('fibers')is the only way to import theFiberconstructor.The
meteor-promisenpm package now has its own.d.tsfile to support this usage.