WickyNilliams / enquire.js

Awesome Media Queries in JavaScript
http://wicky.nillia.ms/enquire.js/
MIT License
3.62k stars 269 forks source link

register should always invoke callbacks asynchronously #127

Open nlwillia opened 9 years ago

nlwillia commented 9 years ago

If I register a callback with enquire, I can't be certain when it will be called. The current implementation will run the callback synchronously if the media query happens to apply, but if the query doesn't apply then the callback will run later. This inconsistency can make it harder to integrate an enquire register call than it should be.

enquire.register('some media query', function() {
    console.log('second'); // oops, might run first
});
console.log('first');

An example of where this can be a problem is inside an AngularJS controller. The Angular idiom is to use $scope.$apply to initiate a digest loop inside the callback since it's triggered externally, but this will fail if the callback is invoked synchronously because a digest is already in progress around the execution of the controller function.

angular.module('...').controller('...', function($scope) {
    enquire.register('some media query', function() {
        $scope.$apply(function() { // fails if synchronous because we're already in a digest cycle
            $scope.someProperty = ...;
        });
    });
});

I use Angular as an example, but this really pertains to any situation where the relative execution order of the callback and the surrounding code matters.

A workaround is to wrap the register call in a timeout, but this is unattractive and verbose. It would be better if enquire added the wrapper itself (probably here). This would be a breaking change for any client that depended on the synchronous behavior, but asynchronous is the safer and more conventional style to use, and I think the API would be better for it. I'd be glad to submit a PR if you like.

cmwelsh commented 9 years ago

Don't release Zalgo:

http://blog.izs.me/post/59142742143/designing-apis-for-asynchrony

If you have an API which takes a callback,
and sometimes that callback is called immediately,
and other times that callback is called at some point in the future,
then you will render any code using this API impossible to reason about,
and cause the release of Zalgo.
WickyNilliams commented 9 years ago

Oh man, I can't believe I did this. I even berated a colleague recently for releasing zalgo. Definitely needs fixing