tlewowski/zurvan

Requires a global Promise variable in node

Closed this issue · 8 comments

If I don't expose bluebird as a global variable in my tests, zurvan blows up with a message "Missing dependency: Promise. Reason(s): global.Promise does not exist".

Currently I'm adding this to the top of my tests which use zurvan:

var Promise = global.Promise = require('bluebird'); // Expose bluebird as a global to make zurvan work

It would be nicer if I could let zurvan know what promise library I'm using - I already have to tell it that I'm using bluebird by calling zurvan.interceptTimers({ bluebird: Promise }), could that not be used to tell zurvan to use bluebird for its promises rather than requiring polluting the global scope?

First of all, thank you for using zurvan, I hope it helps you in whatever you're implementing.

Now, about your problem - I agree that hiding global variables like Promise may be useful for testing in separated environment - is that your use case? If yes, then most probably you also want zurvan to be reentrant - right now it is not (since there is only one event queue anyway, and all timer facilities are global, so most apps use it this way instead of from context).
Requiring zurvan as the first module, before overriding globals should be the proper approach in this case, as we really want to use node internal promises.

The bluebird configuration parameter does not make zurvan use the bluebird promises internally - it is used to reset bluebird default scheduler. Without this parameter problem arises when you require bluebird first and then zurvan (which is virtually always the case, as it shouldn't matter, right?) - in such case bluebird continues to use its default scheduler (which, in case of node.js is setImmediate). Since zurvan overrides setImmediate, the global version (zurvan's) should be used instead of the original one (already cached by bluebird) - otherwise waiting for all events to expire (.waitForEmptyQueue()) does not work, and zurvan is useless.

zurvan was meant to use only node default promises internally - I thought about making this a configurable parameter at the beginning, but then several problems arised - most notably, this would allow user to override most important internal mechanism (waiting for event queue to expire all timeouts and intervals) with any other - including ones that are not A+-compatible, or even worse - ones that claim being A+-compatible, but are not (or with issues like scheduling promises by setTimeout etc.). This would sooner or later lead to nasty issues with stacktraces pointing to zurvan internals , since even if I would be able to execute tests for most common Promise libraries (bluebird, Q...), this would not be sufficient.

Unless, of course, I made a list of "supported internal Promise libraries" and make it only partially configurable (like, .interceptTimers({internalPromise: "Q"})), which is also an option.

Could you please provide a little more details about your use case? This would really help me to find proper approach to solve your problem.

My use case is not that I want to hide the builtin Promise global, it's that there is no Promise in the version of node that I'm using.

I'm building functions for AWS Lambda, which uses node 0.10.36. I'm testing my functions locally, using zurvan to test some polling/timeout code, and using the same 0.10.x branch of node as AWS Lambda supports (to ensure I'm testing in the same sort of environment as the functions will eventaully run in).

As far as I'm aware, node 0.10.x does not have a Promise builtin, so all of my existing code imports bluebird with var Promise = require('bluebird');. zurvan is the first library I've pulled in that requires the variable to be made global, and unfortunately unsetting the global Promise after loading zurvan makes zurvan break so I need to leave the global variable set while I'm running my tests. This means that I might forget to load bluebird in one of my files, and it would work fine in my tests (since I've made Promise global for the tests) but then fail at runtime in AWS Lambda.

I'm aware that the bluebird configuration item does not currently make zurvan use bluebird internally automatically, since zurvan still blows up if global promises are missing. I guess I was asking whether the bluebird configuration parameter could be extended to also make zurvan know what promise library to use - but a separate configuration option would be fine too.

I guess essentially, I just want to be able to use zurvan to test my functions without modifying any globals that could affect how my functions run.

I see, you're bound to one provided. That's a pity.

Of course you're right that node 0.10.x does not have built-in Promise support. 0.10 series was not planned to be supported by zurvan, essentially because it lacks support for Promise (setImmediate was introduced in 0.10, so it should be fine).

I believe that in principle it shouldn't be hard to extend the support to 0.10 series of nodejs, but I need to work out the details. Right now I do not have access to my development environment, but I should be able to handle it during the next week.

Essentially, the change that I can propose (and believe to be testable) is to force a custom Promise library in versions of node that do not have a built-in one. Consequently, this would allow users to override promise library in nay other node release, so I need to check out a safe option (docs & proper error message are a minimum).

I totally understand why you don't want to change globals, I'd also prefer to avoid that. Does a call like:
zurvan.interceptTimers({bluebird: Promise, internalPromises: "bluebird"}) look OK enough? I know that it's kindof duplication, but zurvan.interceptTimers({bluebird: Promise}) already has different semantics, so it would be a breaking change.

Alternatively, I can introduce: zurvan.interceptTimers({usedPromises: bluebird}) that would be equivalent to the set of two configuration variables above.

Of course, if you can't wait until next week, pull requests are welcome

Oh, my bad - I didn't read the compatibility page closely enough - I saw it mentioned "X since Node.js 0.10" in the library compatibility section, but completely missed the engine section which doesn't say node 0.10.x is supported. I'm glad you're open to the idea of adding support for node 0.10.x with external libraries.

For me, either syntax works (the double-config form or the single roll-up config option). If I get some spare time I might look into implementing one of those options this week / weekend, although I'm not 100% sure what the cleanest way to test this would be. Will let you know if I get a chance.

Ok, should work starting from 6f23a2b, please check.

I am aware that Travis still complains for version 0.10, and there will be no new release until CI issues are resolved, but these are mainly related to a different scheduling provided by external library - which is actually expected, since that's what this library is for. Tests are failing as they are bound to native Promise implementation, and custom libs do a little different scheduling. Still, I'd prefer to run all tests on all node releases, so I need to figure out how to run these on 0.10 (or disable on this version, but I'd prefer not to).

Anyways, please check whether your feature works -> to provide compatibility zurvan by default uses a new version of bluebird, if no native Promise implementation is available. Still, you are free to change it to any Promise implementation you prefer by configuration parameter: promiseScheduler: <X>. It is independent from bluebird parameter. Try it out ;)

Starting from 0ce9a7e everything should be fine, including CI builds for version 0.10. Please check if it works in your use case - if it does, please close the issue and I'll publish 0.3.0 still today.

Thanks a lot, 0.3.0 works perfectly for my tests.

Once I stopped setting global.Promise, I actually discovered some of my tests in other files were accidentally relying on it, so it's great to be able to use zurvan without global.Promise set.

I had started trying to fix this myself, but got stuck trying to figure out how to handle Dependencies.js requiring Promise, and how to get the tests working without automatically picking a Promise library to support - I see that zurvan now automatically picks up bluebird as an implementation, which is a much more elegant solution than I had imagined.

Once more, thanks.

I'm glad you like it. Dependency to bluebird is optional, so if for some reason you cannot install it - e.g., private npm repository - it won't fail, just will require additional configuration. Also, it's not used in versions from 0.11 up.

If you encounter any other missing feature that you find useful, please file another issue