{"action":"created","issue":{"url":"https://api.github.com/repos/ericclemmons/react-resolver/issues/2","labels_url":"https://api.github.com/repos/ericclemmons/react-resolver/issues/2/labels{/name}","comments_url":"https://api.github.com/repos/ericclemmons/react-resolver/issues/2/comments","events_url":"https://api.github.com/repos/ericclemmons/react-resolver/issues/2/events","html_url":"https://github.com/ericclemmons/react-resolver/issues/2","id":58375688,"number":2,"title":"Error handling with async resolution","user":{"login":"jesseskinner","id":107450,"avatar_url":"https://avatars.githubusercontent.com/u/107450?v=3","gravatar_id":"","url":"https://api.github.com/users/jesseskinner","html_url":"https://github.com/jesseskinner","followers_url":"https://api.github.com/users/jesseskinner/followers","following_url":"https://api.github.com/users/jesseskinner/following{/other_user}","gists_url":"https://api.github.com/users/jesseskinner/gists{/gist_id}","starred_url":"https://api.github.com/users/jesseskinner/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jesseskinner/subscriptions","organizations_url":"https://api.github.com/users/jesseskinner/orgs","repos_url":"https://api.github.com/users/jesseskinner/repos","events_url":"https://api.github.com/users/jesseskinner/events{/privacy}","received_events_url":"https://api.github.com/users/jesseskinner/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-02-20T16:27:56Z","updated_at":"2015-02-20T20:04:57Z","closed_at":null,"body":"Hey awesome to be collaborating. react-resolver and solve have a different syntax for callbacks, so we should consider which syntax to use.\r\n\r\nsolve doesn't use node-style callbacks (function(error, data)). My thinking here is that if data is being resolved as an object or arrays etc., then only a single value can be emitted from an async resolution, so the callback only takes a single argument. So error handling has to be somehow resolved into the props object.\r\n\r\nOne option might be to have multiple types on a single data point, eg.\r\n\r\n```javascript\r\n{ \r\n    value: function(done) {\r\n       request.get('/api/data', function(error, result) {\r\n            if (error) {\r\n                return done(new Error(\"Failed to load\"));\r\n            }\r\n            return done(result);\r\n       });\r\n    }\r\n}\r\n\r\n// later in render\r\nif (this.props.value instanceof Error) {\r\n    return <Error error={this.props.value} />;\r\n}\r\n// otherwise\r\nreturn <Component value={this.props.value} />;\r\n```\r\n\r\nanother option might be to resolve to multiple properties of an object, eg.\r\n\r\n```javascript\r\n{ \r\n    value: function(done) {\r\n       request.get('/api/data', function(error, result) {\r\n            if (error) {\r\n                return done({ error: new Error(\"Failed to load\"), data: null });\r\n            }\r\n            return done({ error: null, data: result });\r\n       });\r\n    }\r\n}\r\n\r\n// later in render\r\nif (this.props.value.error !== null) {\r\n    return <Error error={this.props.value.error} />;\r\n}\r\n// otherwise\r\nreturn <Component value={this.props.value.data} />;\r\n```\r\n\r\nThoughts on this? Do you think the node/async (error, data) callback syntax is important to maintain? Or do you agree that it make sense to force the user to think about how to translate the errors into a props object in a more deliberate way?\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/ericclemmons/react-resolver/issues/comments/75309498","html_url":"https://github.com/ericclemmons/react-resolver/issues/2#issuecomment-75309498","issue_url":"https://api.github.com/repos/ericclemmons/react-resolver/issues/2","id":75309498,"user":{"login":"jesseskinner","id":107450,"avatar_url":"https://avatars.githubusercontent.com/u/107450?v=3","gravatar_id":"","url":"https://api.github.com/users/jesseskinner","html_url":"https://github.com/jesseskinner","followers_url":"https://api.github.com/users/jesseskinner/followers","following_url":"https://api.github.com/users/jesseskinner/following{/other_user}","gists_url":"https://api.github.com/users/jesseskinner/gists{/gist_id}","starred_url":"https://api.github.com/users/jesseskinner/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jesseskinner/subscriptions","organizations_url":"https://api.github.com/users/jesseskinner/orgs","repos_url":"https://api.github.com/users/jesseskinner/repos","events_url":"https://api.github.com/users/jesseskinner/events{/privacy}","received_events_url":"https://api.github.com/users/jesseskinner/received_events","type":"User","site_admin":false},"created_at":"2015-02-20T20:04:57Z","updated_at":"2015-02-20T20:04:57Z","body":"I like the idea of using `propTypes` to know when it's safe to render. Couldn't it use the actual `propTypes` on the component, rather than something that is the \"equivalent of `propTypes`\"?\r\n\r\nThis sounds good:\r\n\r\n> Dynamic props return a promise or call done(value), where this.props.myProp = value;.\r\n\r\nFor errors not caught within the statics definition, or that should be handled globally, I think this makes a lot of sense:\r\n\r\n> Errors could be handled via resolver.handle(Handler).catch(...) on the server.\r\n\r\nIf users want the errors to be available as props, they could use the mechanisms I described above to catch the errors as they happen, and use the done callback to expose them as part of the prop resolution.\r\n\r\nSo this would give users two ways to handle errors - inline (via callback or promises), and globally (via handler catch), and without adding new API to react-resolver. So the event handling of an `onError` callback is probably superfluous, because it can be done by the user themselves with code in the `catch`.\r\n\r\nRegarding this suggestion:\r\n\r\n> Errors could be available via this.context.resolver.errors.\r\n\r\nI think we should avoid introducing a third pathway for data, there are already `props` and `state` so having users access stuff on `context` is probably unnecessary. Also, I believe `context` is undocumented and might likely change in the future.\r\n"}}