{"action":"created","issue":{"url":"https://api.github.com/repos/nodejs/promises/issues/16","repository_url":"https://api.github.com/repos/nodejs/promises","labels_url":"https://api.github.com/repos/nodejs/promises/issues/16/labels{/name}","comments_url":"https://api.github.com/repos/nodejs/promises/issues/16/comments","events_url":"https://api.github.com/repos/nodejs/promises/issues/16/events","html_url":"https://github.com/nodejs/promises/issues/16","id":133880265,"number":16,"title":"How promise APIs would look like in core","user":{"login":"balupton","id":61148,"avatar_url":"https://avatars.githubusercontent.com/u/61148?v=3","gravatar_id":"","url":"https://api.github.com/users/balupton","html_url":"https://github.com/balupton","followers_url":"https://api.github.com/users/balupton/followers","following_url":"https://api.github.com/users/balupton/following{/other_user}","gists_url":"https://api.github.com/users/balupton/gists{/gist_id}","starred_url":"https://api.github.com/users/balupton/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/balupton/subscriptions","organizations_url":"https://api.github.com/users/balupton/orgs","repos_url":"https://api.github.com/users/balupton/repos","events_url":"https://api.github.com/users/balupton/events{/privacy}","received_events_url":"https://api.github.com/users/balupton/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/nodejs/promises/labels/blocks-landing-pr","name":"blocks-landing-pr","color":"e11d21"},{"url":"https://api.github.com/repos/nodejs/promises/labels/concerns-core-apis","name":"concerns-core-apis","color":"fad8c7"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":43,"created_at":"2016-02-16T04:09:07Z","updated_at":"2016-02-17T19:17:57Z","closed_at":null,"body":"Proposing this thread to consolidate the various discussions and proposals around \"How promise APIs would look like in core.\" as the original pull request covers more than just that sole issue, making it hard to keep track of what is actually being proposed. Feel free to edit. [CD: Thanks!]\r\n\r\n## Proposals for current APIs:\r\n\r\nActive proposals:\r\n\r\n- `require('fs').readFilePromise` (current PR proposal) + `require('fs').promised.readFile` shortcut <br/> https://github.com/nodejs/node/pull/5020#issuecomment-183980246\r\n- `require('fs/promise').readFile` (future PR proposal) (to immediately follow 5020 once landed)\r\n\r\nCurrent / unresolved discussions:\r\n\r\n- do any of the active proposals support callbacks at all? if so, when, why and how? if not, why or when?\r\n- `require('fs/promise').readFile` initially, or  `require('fs').readFilePromise` initially\r\n  - arguments regarding `require('fs/promise').readFile` initially and perhaps only\r\n    - +1 works better with ES6 imports https://github.com/nodejs/node/pull/5020#issuecomment-184016883\r\n    - ~~+1 allows blacklisting https://github.com/nodejs/promises/issues/16#issuecomment-184696679~~ [CD: For top level methods, yes, but not for class instance methods. Still best to block promises using `global.Promise = null` in all cases.]\r\n    - ~~-1 requires PR to be updated https://github.com/nodejs/promises/issues/16#issuecomment-184592603~~ [CD: I am not concerned about changing the PR.]\r\n    - [CD: my concerns with blocking the merge of 5020 on this approach.]\r\n      - -1 unclear on what to do for instance methods (`net.Socket#setTimeout`)\r\n      - -1 unclear on whether or not to re-export non-promise-returning methods (`fs.createReadStream`)\r\n      - -1 difficult to drive consensus within collaborator group on whether or not to add submodule\r\n      - -1 difficult to drive consensus on what to call the module (`promised` is a frontrunner, since we have the name, but this is a potential for bikeshedding)\r\n  - arguments regarding `require('fs').readFilePromise` with or without  `require('fs').promised.readFile` shortcut initially\r\n    - -1 doesn't work well with ES6 imports https://github.com/nodejs/node/pull/5020#issuecomment-184016883\r\n      - [CD: the `promised` shortcut works for destructuring assign, `import x as y` syntax should work for ES2015 modules].\r\n    - -1 despite being in the PR, no consensus has formed yet, also if support is added for all three (not just `require('fs/promise')` then maintaining 3 endpoints that all do the same thing is pain https://github.com/nodejs/node/pull/5020#issuecomment-179452337\r\n    - +1 already implemented in the PR so can be merged in sooner https://github.com/nodejs/promises/issues/16#issuecomment-184592603\r\n    - +1 solution extends to methods of class instances\r\n  - are there any arguments differentiating between  `require('fs').readFilePromise` and  `require('fs').promised.readFile`, why one or the other?\r\n- should promises wrap callback functions, or should callback functions wrap promise functions (initially or later) \r\n\r\nClosed proposals:\r\n\r\n- Promise only: `require('fs').readFile` returns promises only, no support for callback — one API endpoint to maintain, b/c break for everyone, consistent return types, promises are primary and callbacks are eliminated, could provide cleanest forward API\r\n  - argument against here https://github.com/nodejs/promises/issues/16#issuecomment-184726740 b/c break is just too intense\r\n- Callback or promise: `require('fs').readFile` returns promise if no callback is provided, if callback is provided do not return promise and use callback — one API endpoint to maintain, performance hit with additional if check, different return types, callbacks or promises become secondary to the other\r\n  - argument against here https://github.com/nodejs/node/pull/5020#discussion_r51464838 as it overloads return values\r\n  - argument against here https://github.com/nodejs/node/pull/5020#discussion_r51466441 as it will cause issues with things like `process.send(message[, sendHandle][, callback])` which can have a return value despite having an async callback, changing return value to promise will break b/c with current return value\r\n- Promise always with callback support: `require('fs').readFile` returns promise always, if callback is provided attach it to the promise — one API endpoint to maintain, performance hit for callback users, consistent return types, promises are primary and callbacks become secondary\r\n  - argument against here https://github.com/nodejs/node/pull/5020#discussion_r51466441 as it will cause b/c breaks with things like `process.send(message[, sendHandle][, callback])` which can have a return value despite having an async callback, changing return value to promise will break b/c with current return value\r\n\r\n## Proposals for error APIs:\r\n\r\nDiscussion ongoing at #10, summary:\r\n\r\n[CD - There may not need to be an error API — it might look like a flag, like `--abort-on-sync-rejection`]\r\n\r\n``` javascript\r\n// https://github.com/nodejs/promises/issues/10#issue-133802245\r\n// recovery object\r\n// [CD - It appears likely that we'll go this route since error symposium folks have raised concerns about it.]\r\nawait fs.readFilePromise('some/file', {\r\n  ENOENT() {\r\n    return null\r\n  }\r\n})\r\n\r\n// normal use:\r\nfs.readFilePromise('some/file')\r\n```\r\n\r\n``` javascript\r\n// https://github.com/nodejs/promises/issues/10#issuecomment-184375707\r\n// [CD - It's unlikely that we'll go this route because I'd like to avoid subclassing Promise.]\r\nlet file = await fs.getFileAsync('dne').recover({\r\n   EISDIR(err) {}\r\n   ENOENT(err) { ... }\r\n});\r\n```\r\n\r\n## Proposals for new APIs:\r\n\r\n[CD - these probably will not land in nodejs/node#5020].\r\n\r\n- `util.promised`: https://github.com/nodejs/promises/issues/5\r\n- `util.toCallback`: https://github.com/nodejs/promises/issues/6\r\n\r\n---\r\n\r\nThe other issue \"How to make sure promises don't get in the way of callback users with core support.\" seems like a sub-topic of \"How promise APIs would look like in core\". So perhaps that should be consolidated here too. [CD - Promises will be added using a promisify wrapper, without affecting the original function. They should not interfere with callback APIs at all.]"},"comment":{"url":"https://api.github.com/repos/nodejs/promises/issues/comments/185358594","html_url":"https://github.com/nodejs/promises/issues/16#issuecomment-185358594","issue_url":"https://api.github.com/repos/nodejs/promises/issues/16","id":185358594,"user":{"login":"petkaantonov","id":829734,"avatar_url":"https://avatars.githubusercontent.com/u/829734?v=3","gravatar_id":"","url":"https://api.github.com/users/petkaantonov","html_url":"https://github.com/petkaantonov","followers_url":"https://api.github.com/users/petkaantonov/followers","following_url":"https://api.github.com/users/petkaantonov/following{/other_user}","gists_url":"https://api.github.com/users/petkaantonov/gists{/gist_id}","starred_url":"https://api.github.com/users/petkaantonov/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/petkaantonov/subscriptions","organizations_url":"https://api.github.com/users/petkaantonov/orgs","repos_url":"https://api.github.com/users/petkaantonov/repos","events_url":"https://api.github.com/users/petkaantonov/events{/privacy}","received_events_url":"https://api.github.com/users/petkaantonov/received_events","type":"User","site_admin":false},"created_at":"2016-02-17T19:17:57Z","updated_at":"2016-02-17T19:17:57Z","body":"what about \"promised/net\" that returns \"net\" while adding promise methods on the first require?"}}