{"action":"created","issue":{"url":"https://api.github.com/repos/VulcanJS/Vulcan/issues/1949","repository_url":"https://api.github.com/repos/VulcanJS/Vulcan","labels_url":"https://api.github.com/repos/VulcanJS/Vulcan/issues/1949/labels{/name}","comments_url":"https://api.github.com/repos/VulcanJS/Vulcan/issues/1949/comments","events_url":"https://api.github.com/repos/VulcanJS/Vulcan/issues/1949/events","html_url":"https://github.com/VulcanJS/Vulcan/issues/1949","id":312166650,"number":1949,"title":"Callback hooks issue with async/await","user":{"login":"SachaG","id":358832,"avatar_url":"https://avatars3.githubusercontent.com/u/358832?v=4","gravatar_id":"","url":"https://api.github.com/users/SachaG","html_url":"https://github.com/SachaG","followers_url":"https://api.github.com/users/SachaG/followers","following_url":"https://api.github.com/users/SachaG/following{/other_user}","gists_url":"https://api.github.com/users/SachaG/gists{/gist_id}","starred_url":"https://api.github.com/users/SachaG/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SachaG/subscriptions","organizations_url":"https://api.github.com/users/SachaG/orgs","repos_url":"https://api.github.com/users/SachaG/repos","events_url":"https://api.github.com/users/SachaG/events{/privacy}","received_events_url":"https://api.github.com/users/SachaG/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-04-07T03:32:58Z","updated_at":"2018-05-15T03:12:54Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"Currently a `collection.before.sync` callback gets called like this in `mutator.js`:\r\n\r\n```\r\nmodifier = await runCallbacks(`${collectionName}.edit.before`, modifier, document, ...);\r\n```\r\n\r\nSo any async/await code inside a callback hook is resolved at the mutator level.\r\n\r\nThe issue is that if two hooks both have async/await code, since the promise only resolves at the mutator level, it won't resolve *in between* hooks. Meaning that the second hook will receive a Promise as its `modifier` argument instead of a normal object.\r\n\r\nThe solution would be to `async/await` inside `callbacks.js`, where `runCallbacks` is defined. But the problem is that `runCallbacks` is used in many places where it *can't* (or shouldn't) be async, and handling async properly in all those places would be a big refactoring. \r\n\r\nFor now an easy enough workaround is to only use a single async callback on each hook, but it's not a great long-term solution. "},"comment":{"url":"https://api.github.com/repos/VulcanJS/Vulcan/issues/comments/389029430","html_url":"https://github.com/VulcanJS/Vulcan/issues/1949#issuecomment-389029430","issue_url":"https://api.github.com/repos/VulcanJS/Vulcan/issues/1949","id":389029430,"user":{"login":"SachaG","id":358832,"avatar_url":"https://avatars3.githubusercontent.com/u/358832?v=4","gravatar_id":"","url":"https://api.github.com/users/SachaG","html_url":"https://github.com/SachaG","followers_url":"https://api.github.com/users/SachaG/followers","following_url":"https://api.github.com/users/SachaG/following{/other_user}","gists_url":"https://api.github.com/users/SachaG/gists{/gist_id}","starred_url":"https://api.github.com/users/SachaG/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SachaG/subscriptions","organizations_url":"https://api.github.com/users/SachaG/orgs","repos_url":"https://api.github.com/users/SachaG/repos","events_url":"https://api.github.com/users/SachaG/events{/privacy}","received_events_url":"https://api.github.com/users/SachaG/received_events","type":"User","site_admin":false},"created_at":"2018-05-15T03:12:54Z","updated_at":"2018-05-15T03:12:54Z","author_association":"CONTRIBUTOR","body":"> I'm guessing the runs property is actually indicating if the callback is being called with runCallbacks or runCallbacksAsync. If this is the case, I think right with this new runCallbacks behavior is misleading: there are hooks being called with await, hence accept async callbacks, that are marked with runs: 'sync' (examples: new.before, new.sync, new.after, edit.before, edit.sync, edit.after).\r\n\r\nYep, that's right. \r\n\r\n> should runCallbacksAsync be renamed to avoid confusion? I think it should, since what it essentially does is to run callbacks in parallel.\r\n\r\nWe could rename it while keeping `runCallbacksAsync` available as an alias for backwards compatibility?\r\n\r\n> should runCallbacksAsync still be only valid on the server? I guess this decision was taken because the use of Meteor.defer, but now with Promise.all it can also be used in the client\r\n\r\nI haven't really run into use cases where I'd need it on the client but if it makes the API easier to understand why not. I'm not against it. "}}
{"actor":{"display_login":"SachaG"}}