{"action":"created","issue":{"url":"https://api.github.com/repos/nodejs/node/issues/5020","repository_url":"https://api.github.com/repos/nodejs/node","labels_url":"https://api.github.com/repos/nodejs/node/issues/5020/labels{/name}","comments_url":"https://api.github.com/repos/nodejs/node/issues/5020/comments","events_url":"https://api.github.com/repos/nodejs/node/issues/5020/events","html_url":"https://github.com/nodejs/node/pull/5020","id":130309320,"number":5020,"title":"Adding Core support for Promises","user":{"login":"chrisdickinson","id":37303,"avatar_url":"https://avatars.githubusercontent.com/u/37303?v=3","gravatar_id":"","url":"https://api.github.com/users/chrisdickinson","html_url":"https://github.com/chrisdickinson","followers_url":"https://api.github.com/users/chrisdickinson/followers","following_url":"https://api.github.com/users/chrisdickinson/following{/other_user}","gists_url":"https://api.github.com/users/chrisdickinson/gists{/gist_id}","starred_url":"https://api.github.com/users/chrisdickinson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chrisdickinson/subscriptions","organizations_url":"https://api.github.com/users/chrisdickinson/orgs","repos_url":"https://api.github.com/users/chrisdickinson/repos","events_url":"https://api.github.com/users/chrisdickinson/events{/privacy}","received_events_url":"https://api.github.com/users/chrisdickinson/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/nodejs/node/labels/semver-major","name":"semver-major","color":"fbca04"}],"state":"open","locked":false,"assignee":{"login":"chrisdickinson","id":37303,"avatar_url":"https://avatars.githubusercontent.com/u/37303?v=3","gravatar_id":"","url":"https://api.github.com/users/chrisdickinson","html_url":"https://github.com/chrisdickinson","followers_url":"https://api.github.com/users/chrisdickinson/followers","following_url":"https://api.github.com/users/chrisdickinson/following{/other_user}","gists_url":"https://api.github.com/users/chrisdickinson/gists{/gist_id}","starred_url":"https://api.github.com/users/chrisdickinson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chrisdickinson/subscriptions","organizations_url":"https://api.github.com/users/chrisdickinson/orgs","repos_url":"https://api.github.com/users/chrisdickinson/repos","events_url":"https://api.github.com/users/chrisdickinson/events{/privacy}","received_events_url":"https://api.github.com/users/chrisdickinson/received_events","type":"User","site_admin":false},"milestone":null,"comments":432,"created_at":"2016-02-01T09:35:41Z","updated_at":"2016-02-14T19:16:18Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/nodejs/node/pulls/5020","html_url":"https://github.com/nodejs/node/pull/5020","diff_url":"https://github.com/nodejs/node/pull/5020.diff","patch_url":"https://github.com/nodejs/node/pull/5020.patch"},"body":"**Update 02/02/16 15:07:00 PST**: [Current state of the PR.](https://github.com/nodejs/node/pull/5020#issuecomment-178876755)\r\n**Update 02/01/16 14:10:00 PST**: [Current state of the PR.](https://github.com/nodejs/node/pull/5020#issuecomment-178223110)\r\n\r\n**Newcomers**: As a heads up, GitHub has [started clipping the thread down](https://cloud.githubusercontent.com/assets/37303/12769711/ea7538ce-c9ce-11e5-8762-d24798047a48.png). I'm going to start cataloguing common questions tonight, and I'll include them under this notice so that you can quickly look to see if there's an answer to your issue without having to search through the entire thread. Thanks for taking a look & weighing in! Whether you're for this change or against it, your feedback is valuable and I'll respond to it as soon as I'm able.\r\n\r\n## FAQ / TL;DR for newcomers\r\n\r\n#### **What is the current proposal?**\r\n\r\n_If_ landed, this will land **behind a flag**, moving out from behind the flag in the next major version as an unsupported API, finally becoming fully supported one major version after that. It **does not replace or supersede** the callback API, which will continue to be the canonical Node interface.\r\n\r\nPromise-returning variants will be exposed for all \"single-operation\" Node methods — that is to say, Streams and EventEmitter-based APIs are not included in this discussion.\r\n\r\nTo use promises:\r\n\r\n```js\r\nconst fs = require('fs')\r\n\r\nfs.readFile.promise(__filename).then(data => {\r\n})\r\n```\r\n\r\nUsing promises with destructuring assignment (where available):\r\n\r\n```js\r\nconst {readFile} = require('fs').promise\r\n\r\nreadFile(__filename).then(data => {\r\n})\r\n```\r\n\r\nAs a prerequisite to this PR landing, domains must work as expected with native Promises (they currently do not.) AsyncWrap is not blocked on this PR, neither is this PR blocked on AsyncWrap. Both PRs are blocked on getting adequate instrumentation of the microtask queue from V8 (we need a callback for \"microtask enqueued\" events so we can track their creation, and _ideally_ a way to step the microtask queue forward ourselves, running code between invocations.)\r\n\r\n-----------\r\n\r\n#### **Why use the promisify approach?**\r\n\r\nResponse [summed up here](https://github.com/nodejs/node/pull/5020#issuecomment-178112150).\r\n\r\n-----------\r\n\r\n#### **Why not `fs.readFileAsync`?**\r\n\r\n@phpnode notes [that it could break existing users of `bluebird.promisifyAll`](https://github.com/nodejs/node/pull/5020#issuecomment-178236803). Plus, the naming is [contentious](https://github.com/nodejs/node/pull/5020#issuecomment-177889777).\r\n\r\n-----------\r\n\r\n#### **Why not return promises when callbacks are not given?**\r\n\r\n* @jasnell expresses [deep discomfort](https://github.com/nodejs/node/pull/5020#discussion_r51464838) with switching return types.\r\n  * We already have to provide alternate APIs in some places due to \r\n\r\n-----------\r\n\r\n#### **Should programmer errors throw, or reject?**\r\n\r\nSummed up [here](https://github.com/nodejs/node/pull/5020#issuecomment-178189824).\r\n\r\n-----------\r\n\r\n#### **What is the value in adding them to core?**\r\n\r\n@rvagg has been massively helpful in driving this conversation forward:\r\n\r\n* [What are the killer arguments for including this, what is the value in adding this vs. leaving it userland?](https://github.com/nodejs/node/pull/5020#issuecomment-178361191),\r\n  * @domenic notes that [`async/await` is spec'd and roadmapped](https://github.com/nodejs/node/pull/5020#issuecomment-178367909).\r\n  * [My response](https://github.com/nodejs/node/pull/5020#issuecomment-178402443).\r\n  * @mikeal notes [that promises are part of the platform already, and that adding support increases the base of folks willing to help fix bugs with in interoperation with them](https://github.com/nodejs/node/pull/5020#issuecomment-178727217)\r\n* @rvagg [asks for clarification on the value](https://github.com/nodejs/node/pull/5020#issuecomment-179146506) as a part of this comment.\r\n  * [My (wordy, sorry) response](https://github.com/nodejs/node/pull/5020#issuecomment-179186510).\r\n\r\n-----------\r\n\r\n#### **How do you address the need to keep core small while expanding the exposed API?**\r\n#### **Where do we draw the line in supporting language-level constructs?**\r\n\r\n@yoshuawuyts, @rvagg, and @DonutEspresso have largely driven this conversation:\r\n\r\n* @yoshuawuyts [asks](https://github.com/nodejs/node/pull/5020#issuecomment-178887233).\r\n  * @mikeal [responds](https://github.com/nodejs/node/pull/5020#issuecomment-178891923).\r\n  * @brianleroux [responds](https://github.com/nodejs/node/pull/5020#issuecomment-178892671).\r\n  * [My response](https://github.com/nodejs/node/pull/5020#issuecomment-178894842).\r\n* @DonutEspresso [raises concerns](https://github.com/nodejs/node/pull/5020#issuecomment-178926163).\r\n  * [My response](https://github.com/nodejs/node/pull/5020#issuecomment-178942306).\r\n* @rvagg [asks](https://github.com/nodejs/node/pull/5020#issuecomment-179146506) as a part of this comment.\r\n  * [My (wordy, sorry) response](https://github.com/nodejs/node/pull/5020#issuecomment-179186510).\r\n\r\n-----------\r\n\r\n#### **Why are modules (like `require('fs/promise')`) not in scope for this discussion?**\r\n\r\n@phpnode, @RReverser, @mikeal, @ktrott, (and others, I'm sure!) have brought this up, and I've been pushing to have this conversation in a different, later issue. The conversation has mainly revolved around consensus, and the difficulty in attaining it when adding modules to the mix.\r\n\r\n* @ktrott does an [excellent job of summing up the concern](https://github.com/nodejs/node/pull/5020#issuecomment-179452337).\r\n  * My [(again wordy, sorry!) response](https://github.com/nodejs/node/pull/5020#issuecomment-179505468).\r\n\r\n-------------\r\n\r\n**This is the original text of the PR.** It does not reflect the current state of the conversation. See above for common themes!\r\nThis PR introduces Core support for Promises. Where possible, callbacks have\r\nbeen made optional; when omitted, a `Promise` is returned. For some APIs (like\r\n`http.get` and `crypto.randomBytes`) this approach is not feasible. In those\r\ncases promise-returning `*Async` versions have been added\r\n(`crypto.randomBytesAsync`). The naming there isn't set in stone, but it's\r\nbased on how [`bluebird`][ref-bluebird]'s [`promisifyAll`][ref-promisify-all]\r\nmethod generates names.\r\n\r\nThis PR allows users to swap the *implementation* of promises used by core.\r\nThis helps in a few dimensions:\r\n\r\n* Folks that prefer a particular flavor of Promises for sugar methods can set\r\n  the promise implementation at app-level and forget about it.\r\n* This should alleviate concerns about stifling ecosystem creativity — putting\r\n  promises in Core doesn't mean Core needs to pick a winner.\r\n* Folks who prefer a faster implementation of promises can swap in their\r\n  preferred library; folks who want to use whatever native debugging is added\r\n  at the V8 level for native promises can avoid swapping in promises.\r\n* A world of benchmarks opens up for Promise library authors — any code in the\r\n  ecosystem that uses promises generated by core and has a benchmarking suite\r\n  is a potential benchmark for promise libraries.\r\n\r\nThe API for swapping implementations is `process.setPromiseImplementation()`.\r\nIt may be used many times, **however** after the first time deprecation warnings\r\nare logged with the origin of the *first* call. This way if a package misbehaves\r\nand sets the implementation for an application, it's easy to track down.\r\n\r\nExample:\r\n\r\n```js\r\nprocess.setPromiseImplementation(require('bluebird'));\r\n\r\nconst fs = require('fs');\r\n\r\nfs.readFile('/usr/share/dict/words', 'utf8')\r\n  .then(words => console.log(`there are ${words.split('\\n').length} words`))\r\n  .catch(err => console.error(`there are no words`));\r\n```\r\n\r\nStreams are notably missing from this PR – promisifying streams is more\r\ninvolved since the ecosystem already relies on a common (and more importantly,\r\noften **pinned**) definition of how that type works. Changing streams will take\r\neffort and attention from the @nodejs/streams WG. All other callback-exposing\r\nmodules and methods have been promisified (or an alternative made available),\r\nthough.\r\n\r\nThree new internal modules have been added:\r\n\r\n* `lib/internal/promises` — tracks the original builtin `Promise` object, as \r\n  well as the currently selected `Promise` implementation. All promises \r\n  created by Node are initially native, and then passed to the selected\r\n  implementation's `.resolve` function to cast them.\r\n* `lib/internal/promisify` — turn a callback-accepting function into one that\r\n  accepts callbacks *or* generates promises. Errors thrown on the same tick\r\n  are still thrown, not returned as rejected promises — in other words,\r\n  programmer errors, such as invalid encodings, are still eagerly thrown.\r\n* `lib/internal/callbackify` — turn a synchronous or promise-returning function\r\n  into a callback-accepting function. This is only used in `lib/readline` for\r\n  the `completer` functionality. This could probably fall off this PR, but it\r\n  would be useful for subsequent changes to streams.\r\n\r\n## Breaking Changes\r\n\r\n* In general: any API that would have thrown with no callback most likely does\r\n  **not** throw now.\r\n* `fs` APIs called without a callback will no longer crash the process with an\r\n  exception.\r\n* `ChildProcess#send` with no callback no longer returns `Boolean`, instead\r\n  returns `Promise` that resolves when `.send` has completed.\r\n* Wrapped APIs:\r\n  * `child_process.ChildProcess#send`\r\n  * `cluster.disconnect`\r\n  * `dgram.Socket#bind`\r\n  * `dgram.Socket#close`\r\n  * `dgram.Socket#send`\r\n  * `dgram.Socket#sendto`\r\n  * `dns.lookupService`\r\n  * `dns.lookup`\r\n  * `dns.resolve`\r\n  * `fs.access`\r\n  * `fs.appendFile`\r\n  * `fs.chmod`\r\n  * `fs.chown`\r\n  * `fs.close`\r\n  * `fs.exists`\r\n  * `fs.fchmod`\r\n  * `fs.fchown`\r\n  * `fs.fdatasync`\r\n  * `fs.fstat`\r\n  * `fs.fsync`\r\n  * `fs.ftruncate`\r\n  * `fs.futimes`\r\n  * `fs.lchmod`\r\n  * `fs.lchown`\r\n  * `fs.link`\r\n  * `fs.lstat`\r\n  * `fs.mkdir`\r\n  * `fs.open`\r\n  * `fs.readFile`\r\n  * `fs.readdir`\r\n  * `fs.readlink`\r\n  * `fs.realpath`\r\n  * `fs.rename`\r\n  * `fs.rmdir`\r\n  * `fs.stat`\r\n  * `fs.symlink`\r\n  * `fs.unlink`\r\n  * `fs.utimes`\r\n  * `fs.writeFile`\r\n  * `net.Socket#setTimeout`\r\n  * `readline.Interface#question`\r\n  * `repl.REPLServer#complete`\r\n  * `zlib.deflateRaw`\r\n  * `zlib.deflate`\r\n  * `zlib.gunzip`\r\n  * `zlib.gzip`\r\n  * `zlib.inflateRaw`\r\n  * `zlib.inflate`\r\n  * `zlib.unzip`\r\n\r\n## New APIs\r\n\r\n* **`process.setPromiseImplementation`**\r\n* `net.connectAsync`\r\n* `tls.connectAsync`\r\n* `crypto.randomBytesAsync`\r\n* `crypto.pbkdf2Async`\r\n* `crypto.pseudoRandomBytesAsync`\r\n* `crypto.rngAsync`\r\n* `crypto.prngAsync`\r\n* `http.getAsync`\r\n* `https.getAsync`\r\n* `http.requestAsync`\r\n* `https.requestAsync`\r\n* `child_process.execAsync`\r\n* `child_process.execFileAsync`\r\n\r\n## Next steps\r\n\r\nI haven't finished this PR yet: the primary missing piece is tests for the\r\npromise-based APIs, to ensure that they resolve to the correct values. Docs\r\nalso need updated. I'll be working on this in my free time over the next week.\r\n\r\nHere are the bits that I'd like folks reading this to keep in mind:\r\n\r\n* The code changes here are fairly cheap, time-wise — one evening's worth of work.\r\n  * Technical possibility has never been the primary blocker.\r\n* We know async/await is coming down the pipe, and many devs are interested in\r\n  it.\r\n  * With ChakraCore potentially coming in, this may happen sooner than anyone\r\n    previously imagined.\r\n  * Even sans the ChakraCore PR, it's my understanding that V8 will be supporting\r\n    async/await by EOY (Correct me if I'm wrong, here!)\r\n* **Promises may not be your cup of tea.** This is okay. This is not an attempt to \r\n  **replace** callbacks or streams with promises. They can co-exist. While Promises \r\n  are complicated, much of that complication falls out of the problem space that \r\n  both callbacks and promises abstract over. Give this a fair shake.\r\n\r\n[ref-bluebird]: http://bluebirdjs.com/\r\n[ref-promisify-all]: http://bluebirdjs.com/docs/api/promise.promisifyall.html"},"comment":{"url":"https://api.github.com/repos/nodejs/node/issues/comments/183957804","html_url":"https://github.com/nodejs/node/pull/5020#issuecomment-183957804","issue_url":"https://api.github.com/repos/nodejs/node/issues/5020","id":183957804,"user":{"login":"jasnell","id":439929,"avatar_url":"https://avatars.githubusercontent.com/u/439929?v=3","gravatar_id":"","url":"https://api.github.com/users/jasnell","html_url":"https://github.com/jasnell","followers_url":"https://api.github.com/users/jasnell/followers","following_url":"https://api.github.com/users/jasnell/following{/other_user}","gists_url":"https://api.github.com/users/jasnell/gists{/gist_id}","starred_url":"https://api.github.com/users/jasnell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasnell/subscriptions","organizations_url":"https://api.github.com/users/jasnell/orgs","repos_url":"https://api.github.com/users/jasnell/repos","events_url":"https://api.github.com/users/jasnell/events{/privacy}","received_events_url":"https://api.github.com/users/jasnell/received_events","type":"User","site_admin":false},"created_at":"2016-02-14T19:16:18Z","updated_at":"2016-02-14T19:16:18Z","body":"@chrisdickinson ... Excellent summary. Thank you very much. I know this has\nbeen, and is continuing to be, a difficult process. I think this is heading\nin the right direction, even if there are still many details to work out\nthrough subsequent iterations... That's why it's experimental for now after\nall :) I doubt we'll get it right the first time and there are obviously\nmany issues to work out. We'll get there. +1 on the proposed direction.\nOn Feb 14, 2016 12:55 AM, \"Chris Dickinson\" <notifications@github.com>\nwrote:\n\n> I've written a summary\n> <https://medium.com/@isntitvacant/adding-promise-support-to-core-a4ea895ccbda#.bjc9oixft>\n> of the current state of the PR, including the problems being worked on at\n> present. Thanks all!\n>\n> —\n> Reply to this email directly or view it on GitHub\n> <https://github.com/nodejs/node/pull/5020#issuecomment-183854904>.\n>\n"}}