asynquence icon indicating copy to clipboard operation
asynquence copied to clipboard

Comments on Documentation

Open richb-hanover opened this issue 8 years ago • 10 comments

Update: Changed subject to reflect the current focus.

I tried out the example at https://github.com/getify/asynquence/blob/master/contrib/README.md#errfcb-plugin that talks about sq.errfcb() plugin. When I execute the code, the parameter passed to .then() doesn't seem to have the desired data. (That's the data which would have been passed as the second parameter of the error-first callback). How is it made available?

// first install with "npm install asynquence"
var ASQ = require("asynquence-contrib");
fs = require("fs");

// Node.js: fs.readFile(..) wrapper
function readFile(filename) {
  // setup an empty sequence (much like an empty
  // promise)
  var sq = ASQ();

  // call Node.js' `fs.readFile(..), but pass in
  // an error-first callback that is automatically
  // wired into a sequence
  fs.readFile( filename, 'utf8', sq.errfcb() );

  // now, return our sequence/promise
  return sq;
}

readFile("/Users/richb/meaningoflife.txt")
  .then((data)  =>
    { console.log("Read fine...", data) })
  .or((err) =>
    { console.log("Error", err) })

Update: added 'utf8' parameter to fs.readFile() so that resulting data is a string, not a buffer.

richb-hanover avatar Feb 05 '16 06:02 richb-hanover

Answered my own question. Parameters to .then() must be (done, data)...

//...
readFile("/Users/richb/meaningoflife.txt")
  .then((done, data)  =>
    { console.log("Read fine...", data) })
  .or((err) =>
    { console.log("Error", err) })

It would be good to expand the example to include these lines that consume the data/err results.

richb-hanover avatar Feb 05 '16 06:02 richb-hanover

In ASQ, the then(..) function assumes the following signature for its callbacks:

( triggerDoneCallback, msg1, msg2, ... )

If you want it to behave like a promise#then(..) method -- leave off the triggerDoneCallback and since it only accepts a single arg, all those msg1, msg2, etc have to wrapped into a single array -- then use the pThen(..) method from the pThen contrib plugin.

Alternatively, since you're in this example doing a synchronous step (that is, not needing to return for chaining a sub-sequence) like console.log(..), just use .val(..) instead of .then(..).

To summarize, your 3 options are:

  1. .then( (_, data) => .. )
  2. .pThen( data => .. )
  3. .val( data => .. ) (synchronous steps only)

getify avatar Feb 05 '16 06:02 getify

heh, cross posting. :)

getify avatar Feb 05 '16 06:02 getify

Thanks for the speedy response. I'm new to promises/ASQ/etc. and don't fully have a sense of how to "speak in Promises" versus how to "speak in asynquence".

Here are a couple comments about things that made it hard for me as a newcomer to get started.

  • For those who are trying to follow the basic pattern (without fancy things such as 2) or 3) above), it would help for the doc's to be a bit more prescriptive, and maybe have a fully-worked example.

  • As I noted above, it would have helped me if those final lines of the example showed how to consume the err and data from the original callback.

  • It would be helpful to make an explicit description of how to install/include asynquence and the contrib plugins. I tried many things, and my example above is what finally worked. I also think I found one of the examples seemed to imply the following, but I don't think it worked.

    var ASQ = require("asynquence");
    require("asynquence-contrib");
    
  • I think the doc's also imply that asynquence-contrib was within the node_modules/asynquence folder. It is no longer there.

NB I also think I got hung up on a npm version problem. asynquence and/or asynquence-contrib didn't install properly, and upgrading npm from 3.3.12 to 3.7.1 did seem to fix things.

Thanks again!

richb-hanover avatar Feb 05 '16 06:02 richb-hanover

this is great feedback on improving docs, appreciated.

however, you've raised several substantial/important issues that need to be addressed. I would prefer you file these as separate issues so we can track them:

  1. " .. how to install/include asynquence and the contrib plugins .. I found one of the examples .. but I don't think it worked." This is definitely a problem. It should work that way, and it does in all my usage. We need to figure out why it's not working for you. Please file an issue with exact reproduce details.
  2. "..the doc's also imply that asynquence-contrib was within the node_modules/asynquence folder." I'm not aware of the docs saying that. It was never the case. Can you file an issue with details about where the docs say that so we can clarify/correct?
  3. "..asynquence and/or asynquence-contrib didn't install properly, and upgrading npm from 3.3.12 to 3.7.1 did seem to fix things." This should not be the case currently. There was a period of time several months back where npm3 had come out and changed its rules for how it deals with packages and versions, and asynquence and asynquence-contrib were in fact uninstallable. I released updated versions of both packages that fixed that problem. Also, npm changed their code to stop making some of those breakages. Again, we need to file a separate issue with specific info about how you are installing and what errors are happening. They should not be.

Thanks!

getify avatar Feb 05 '16 06:02 getify

OK. I will take some time to see what I can re-create and file separate bugs as I nail down test cases. (I was in the throes of starting with new technology, and consequently, not paying full attention to what was happening.) Thanks again for your strong support.

richb-hanover avatar Feb 05 '16 14:02 richb-hanover

Responding to the items above: I believe the doc's are correct - my troubles with installation came from misreading the docs. A couple things would make it better: a fully-worked example (see 1. below), and a more clear description of require() (2. below)

  1. I like that there are a lot of examples. But a fully worked example (like the one in the original post above including a comment to "npm install asynquence" and require('asynquence') early in the README would give newbies confidence about getting started.
  2. About item #2 above, I was confused about the description of requiring asynquence starting at https://github.com/getify/asynquence#plugin-extensions. To fix that...
  3. I think the documentation would benefit from factoring out the contrib items into contrib/README.md. Also change "Plugin Extensions" heading to "Contributed Plugin Extensions". The intro para should give a link to contrib/README.md, which might have its own description of var ASQ = require('asynquence-contrib'); with an explanation that this will automatically pull in asynquence (This was part of the problem I had thinking that asynquence-contrib was in the same folder as asynquence.)
  4. I don't think I can reproduce the difficulties with installation (sorry.) I have now updated npm, and it's working fine.

TL;DR - I think the tweaks above to the documentation would resolve all these problems (except 4.) Thanks.

richb-hanover avatar Feb 10 '16 02:02 richb-hanover

Thanks again, fantastic feedback. Will try to digest and make changes as I find some time. :)

getify avatar Feb 10 '16 06:02 getify

also... s/passsed/passed/g

richb-hanover avatar Feb 17 '16 20:02 richb-hanover

Also look at #93.

legodude17 avatar Jul 29 '16 18:07 legodude17