{"action":"created","issue":{"url":"https://api.github.com/repos/repeaterjs/repeater/issues/48","repository_url":"https://api.github.com/repos/repeaterjs/repeater","labels_url":"https://api.github.com/repos/repeaterjs/repeater/issues/48/labels{/name}","comments_url":"https://api.github.com/repos/repeaterjs/repeater/issues/48/comments","events_url":"https://api.github.com/repos/repeaterjs/repeater/issues/48/events","html_url":"https://github.com/repeaterjs/repeater/issues/48","id":542601617,"node_id":"MDU6SXNzdWU1NDI2MDE2MTc=","number":48,"title":"Not that great for implementing transducers?","user":{"login":"jedwards1211","id":1448194,"node_id":"MDQ6VXNlcjE0NDgxOTQ=","avatar_url":"https://avatars0.githubusercontent.com/u/1448194?v=4","gravatar_id":"","url":"https://api.github.com/users/jedwards1211","html_url":"https://github.com/jedwards1211","followers_url":"https://api.github.com/users/jedwards1211/followers","following_url":"https://api.github.com/users/jedwards1211/following{/other_user}","gists_url":"https://api.github.com/users/jedwards1211/gists{/gist_id}","starred_url":"https://api.github.com/users/jedwards1211/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jedwards1211/subscriptions","organizations_url":"https://api.github.com/users/jedwards1211/orgs","repos_url":"https://api.github.com/users/jedwards1211/repos","events_url":"https://api.github.com/users/jedwards1211/events{/privacy}","received_events_url":"https://api.github.com/users/jedwards1211/received_events","type":"User","site_admin":false},"labels":[{"id":1588012637,"node_id":"MDU6TGFiZWwxNTg4MDEyNjM3","url":"https://api.github.com/repos/repeaterjs/repeater/labels/documentation","name":"documentation","color":"a9f73b","default":true,"description":"Improvements to docs are requested"},{"id":1308233463,"node_id":"MDU6TGFiZWwxMzA4MjMzNDYz","url":"https://api.github.com/repos/repeaterjs/repeater/labels/question","name":"question","color":"d876e3","default":true,"description":"Further information is requested"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":11,"created_at":"2019-12-26T16:42:13Z","updated_at":"2019-12-26T20:41:50Z","closed_at":null,"author_association":"NONE","body":"Since the API is so similar to Observable, I find it great for consuming observable-like things but I'm not as sold for using it on non-observable-like things, like transducers or other async iterator combiners.\r\n\r\nIt seems almost simpler to implement transducers like `map` correctly using a raw async iterator protocol than using `Repeater`.\r\n\r\nThe naive assumption (in `repeaterMap` below) is to use a `for await` loop, but this keeps consuming input after the caller returns, which would be especially bad if the input is infinite.  Whereas a raw async iterator can immediately forward return and throw calls to the input.\r\n\r\nA less naive attempt (`awkwardRepeaterMap`) is to do the `for await` in a separate function and break once stop is detected.  However more calls to `next()` still go through than with the raw async iterator.\r\n\r\nThoughts?\r\n\r\n### Code\r\n```js\r\nconst { Repeater } = require('@repeaterjs/repeater')\r\n\r\nconst range = n => ({\r\n  i: 0,\r\n  n,\r\n  async next() {\r\n    console.log('next()')\r\n    const i = ++this.i\r\n    if (i >= this.n) return { done: true }\r\n    return { value: i, done: false }\r\n  },\r\n  async return() {\r\n    console.log('return()')\r\n    this.i = this.n\r\n    return { done: true }\r\n  },\r\n  async throw(error) {\r\n    console.log('throw()')\r\n    this.i = this.n\r\n    return { done: true }\r\n  },\r\n  [Symbol.asyncIterator]() {\r\n    return this\r\n  },\r\n})\r\n\r\nconst repeaterMap = (iterable, iteratee) =>\r\n  new Repeater(async (push, stop) => {\r\n    for await (const value of iterable) push(iteratee(value))\r\n    await stop\r\n  })\r\n\r\nconst rawMap = (iterable, iteratee) => ({\r\n  iterator: iterable[Symbol.asyncIterator](),\r\n  async next() {\r\n    const { value, done } = await this.iterator.next()\r\n    return { value: iteratee(value), done }\r\n  },\r\n  return() {\r\n    return this.iterator.return()\r\n  },\r\n  throw(error) {\r\n    return this.iterator.throw(error)\r\n  },\r\n  [Symbol.asyncIterator]() {\r\n    return this\r\n  },\r\n})\r\n\r\n\r\nconst awkwardRepeaterMap = (iterable, iteratee) =>\r\n  new Repeater(async (push, stop) => {\r\n    let stopped = false\r\n    async function next() {\r\n      for await (const value of iterable) {\r\n        if (stopped) break\r\n        push(iteratee(value))\r\n      }\r\n    }\r\n    next()\r\n    await stop\r\n    stopped = true\r\n  })\r\n\r\nasync function go() {\r\n  console.log('rawMap')\r\n  for await (const i of rawMap(range(10), i => i * 2)) {\r\n    console.log(i)\r\n    if (i >= 5) break\r\n  }\r\n  console.log('\\nrepeaterMap')\r\n  for await (const i of repeaterMap(range(10), i => i * 2)) {\r\n    console.log(i)\r\n    if (i >= 5) break\r\n  }\r\n  console.log('\\nawkwardRepeaterMap')\r\n  for await (const i of awkwardRepeaterMap(range(10), i => i * 2)) {\r\n    console.log(i)\r\n    if (i >= 5) break\r\n  }\r\n}\r\n\r\ngo()\r\n```\r\n\r\n### Output\r\n```\r\nrawMap\r\nnext()\r\n2\r\nnext()\r\n4\r\nnext()\r\n6\r\nreturn()\r\n\r\nrepeaterMap\r\nnext()\r\nnext()\r\nnext()\r\n2\r\nnext()\r\nnext()\r\nnext()\r\n4\r\nnext()\r\nnext()\r\n6\r\nnext()\r\nnext()\r\n\r\nawkwardRepeaterMap\r\nnext()\r\nnext()\r\nnext()\r\n2\r\nnext()\r\nnext()\r\nnext()\r\n4\r\nnext()\r\nnext()\r\n6\r\nnext()\r\nreturn()\r\n```"},"comment":{"url":"https://api.github.com/repos/repeaterjs/repeater/issues/comments/569130398","html_url":"https://github.com/repeaterjs/repeater/issues/48#issuecomment-569130398","issue_url":"https://api.github.com/repos/repeaterjs/repeater/issues/48","id":569130398,"node_id":"MDEyOklzc3VlQ29tbWVudDU2OTEzMDM5OA==","user":{"login":"brainkim","id":2996754,"node_id":"MDQ6VXNlcjI5OTY3NTQ=","avatar_url":"https://avatars1.githubusercontent.com/u/2996754?v=4","gravatar_id":"","url":"https://api.github.com/users/brainkim","html_url":"https://github.com/brainkim","followers_url":"https://api.github.com/users/brainkim/followers","following_url":"https://api.github.com/users/brainkim/following{/other_user}","gists_url":"https://api.github.com/users/brainkim/gists{/gist_id}","starred_url":"https://api.github.com/users/brainkim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/brainkim/subscriptions","organizations_url":"https://api.github.com/users/brainkim/orgs","repos_url":"https://api.github.com/users/brainkim/repos","events_url":"https://api.github.com/users/brainkim/events{/privacy}","received_events_url":"https://api.github.com/users/brainkim/received_events","type":"User","site_admin":false},"created_at":"2019-12-26T20:41:50Z","updated_at":"2019-12-26T20:41:50Z","author_association":"MEMBER","body":"Ahh I see `this.listening` is set to false before `emptyQueue` awaits `this.subscriptionIds`, so it causes `next` calls which were stuck on await `this.subscribeAll` to continue with a call to `this.return` instead of `this.pullValue` according to the conditional expression. However I feel like this is still 1 microtick slower than the original `iter.return` call so the original `iter.return` call should still win the race, right?\r\n\r\n> So you're right that that code could probably be improved\r\n\r\nTo be clear, I wasn’t criticizing the code, I don’t think it’s of poor quality/could be improved, and it seems pretty readable as far as async code which uses then directly goes. My secret wish is that someone takes the repeater codebase and rewrites it, keeping only the unit tests, because I feel like the code has gotten pretty hairy to handle a bunch of important edge-cases."}}
{"actor":{"display_login":"brainkim"}}