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

register should always invoke callbacks asynchronously #127

Open
nlwillia opened this issue Jan 17, 2015 · 2 comments
Open

register should always invoke callbacks asynchronously #127

nlwillia opened this issue Jan 17, 2015 · 2 comments
Labels

Comments

@nlwillia
Copy link

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
Copy link

cmwelsh commented Jan 19, 2015

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants