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

Need a way to inject the nextTick implementation #44

Closed
cadorn opened this issue Feb 4, 2012 · 11 comments
Closed

Need a way to inject the nextTick implementation #44

cadorn opened this issue Feb 4, 2012 · 11 comments
Assignees

Comments

@cadorn
Copy link

cadorn commented Feb 4, 2012

I am using Q in mozilla addon sdk which parses for static requires in order to authorize loading a module dependency from a module.

Problem is serverSideRequire("event-queue") which is not detected (as indended).

The best solution that does not interfere with the current requirements seems to be injecting the nextTick implementation.

Could we add something like Q.nextTick which I can set externally and use that over fallbacks?

@domenic
Copy link
Collaborator

domenic commented Feb 4, 2012

This makes sense. Just to make sure, neither the MessageChannel nor setTimeout implementations are sufficient for your purposes? I would imagine the former is both available and sufficiently fast.

In any case, there is actually already a Q.nextTick, but we'd need to change all the internal usages to reference exports.nextTick instead of just nextTick if we wanted to make setting it useful.

@cadorn
Copy link
Author

cadorn commented Feb 4, 2012

setTimeout() is not available as a global in the mozilla addon environment.

serverSideRequire("event-queue") will return a module I am providing BUT the addon sdk loader will not allow me to load the module without finding it declared in a static analysis pass (looking for require('<id>')).

If I can set exports.nextTick externally I can bypass all the current detection logic.

@domenic
Copy link
Collaborator

domenic commented Feb 4, 2012

A settable nextTick makes sense to me. I'm just wishing there was a way to make it work out of the box. Let me know if you think of anything.

I'll wait for @kriskowal to weigh in before committing anything though, since I don't understand all the security guarantees Q provides enough to know whether a settable nextTick would break them.

@cadorn
Copy link
Author

cadorn commented Feb 5, 2012

Unfortunately there is no clean way to detect the addon sdk environment. Since setTimeout() nor checking for serverSideRequire("timers").setTimeout (due to not finding require('timers')) will work I think the only solution is setting nextTick externally.

The function could be cached on first use to avoid security implications if that helps.

@kriskowal
Copy link
Owner

I would prefer for the nextTick underlying implementation to be injected somehow, rather than patched on post-instantiation. There are no securability issues if it’s injected, but there are security implication if we allow it to be replaced.

Bear in mind that this implementation of the Q interface does not make security guarantees. It just strives to behave identically to a hypothetical secure implementation like ref_send or makeQ. It is intended to pave the way, not be the way. That is, while it does make a lot of guarantees about the order of events (which are good for robustness in general), it does not guarantee that mutually suspicious programs could share the Q interface or share promise objects without fear of interference. To do so, it would have to freeze a lot more.

This is a tricky situation. @cadorn, does the MessageChannel implementation of nextTick not work in the Addon SDK? I am thinking that the event-queue should be entirely subsumed by Q at this point.

@cadorn
Copy link
Author

cadorn commented Feb 6, 2012

@kriskowal typeof MessageChannel: undefined

injection is indeed the way to go and that will be available in future as we have more intelligent loaders but Q is such a low-level module that it may be used without an injecting loader quite frequently (or even to implement the loader as in my case). I don't think the addon-sdk is alone when it comes to striving to provide a clean (minimal globals) module environment.

You can say; just patch Q when you import it into your project; but that is not consistent with the current attempts to subsume the event-queue and provide an out-of-the-box experience.

The addon-sdk exposes require("timers").setTimeout() but if used as serverSideRequire("timers").setTimeout it will not allow access to the timers module as the static analysis fails. (I currently implement an event-queue module and am adding require('event-queue'); to the top of q.js).

To prevent replacing the nextTick implementation it should be cached on first use. I think this is a fair workaround for the lack of injectability.

There are also cases when you want more control over nextTick than Q currently offers. In the addon-sdk the timers implementation respects the lifecycle of the program and test runner. See: https://github.com/mozilla/addon-sdk/blob/master/packages/api-utils/lib/timer.js

I suppose if we decide against a one-time settable nextTick I can look into teaching the addon-sdk loader the ability to allow loading of the event-queue module (since it cannot find it by static analysis) by configuration from package.json. This is a use-case not unique to this scenario but was hoping to hold off on for now.

@kriskowal
Copy link
Owner

@cadorn Does the addon runtime provide a reuqire.async(id, cb, eb) hook?

@cadorn
Copy link
Author

cadorn commented Feb 6, 2012

@kriskowal No. They may if it becomes a commonjs spec though at which point it could be used instead of serverSideRequire.

The problem is they want lean builds to generate addons that only contain what is needed. If using require.async(id, cb, eb) we still have no way to determine what module is being referenced and would need a declaration in package.json. This is a problem area I am working on in general (constraining require.async for a module so we can gather possible IDs at build time in order to generate the dynamic load bundles). I have a working solution but am not happy with it. I think it will end up needing a pureJS load handler / dynamic linker that can be invoked at runtime or build time.

If we don't want to make changes to Q to support the addon-sdk the consequence for addon-sdk developers is:

  • Must use a non-enforcing loader instead of the stock one (I am about to test that by subclassing the stock loader) and
  • Built add-on cannot be a lean build and must include all files.

This is fine for now but will not fly longer term.

@Yaffle
Copy link

Yaffle commented Apr 24, 2012

@domenic
ie 10 supports MessageChannel, does nextTick via MessageChannel works under IE10 ?

@domenic
Copy link
Collaborator

domenic commented Apr 24, 2012

@Yaffle Yes it does, although of course IE10 has native msSetImmediate. See also YuzuJS/setImmediate@9be3252.

@kriskowal
Copy link
Owner

IE10 implemented setImmediate? Neato. Let’s use it.

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

No branches or pull requests

4 participants