Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation example is confusing #2

Open
thejsj opened this issue Aug 5, 2015 · 0 comments
Open

Documentation example is confusing #2

thejsj opened this issue Aug 5, 2015 · 0 comments

Comments

@thejsj
Copy link

thejsj commented Aug 5, 2015

First of all, thanks for doing this! This npm module seems pretty interesting. I personally don't use koa, but just wanted to make a quick suggestion to your documentation.

I like the simplicity of your example, but the variable naming is a bit confusing. Let me try to explain why with comments...

var koa = require('koa')
// for use with koa-generic-session
var session = require('koa-generic-session')
var RethinkSession = require('koa-generic-session-rethinkdb')
/*!
 * In most example of rethinkdbdash, the code used to instanstiate it is the following:
 * `var r = require('rethinkdbdash')();`
 * The fact that this is different (only by two parenthesis is already a little confusing!). 
 * That being said, naming it to `rethinkdb` seems appropriate
 */
var rethinkdb = require('rethinkdbdash')

/*!
 * What the function returned by importing `rethinkdbdash` returns is not really a connection. 
 * It's more an instance of rethinkdbdash which actually has a connection pool, which actually 
 * has multiple connections. Using the word connection is not totally off, but (by convention) 
 * most people use the `r` variable here. Not using the `r` variable somehow seems to suggest that
 * this is different, when this is actually not.
 */ 
var connection = rethinkdb({
  host: 'localhost',
  port: 28015
})

var sessionStore = new RethinkSession({connection: connection})
// create the db, table and indexes to store sessions
sessionStore.setup()

var app = koa()
// used for cookie stuffs
app.keys = ['foo', 'bar']

app.use(session({
  store: sessionStore
})

I suggest the following:

var koa = require('koa')
// for use with koa-generic-session
var session = require('koa-generic-session')
var RethinkSession = require('koa-generic-session-rethinkdb')
var r = require('rethinkdbdash')({
  host: 'localhost',
  port: 28015
});

var sessionStore = new RethinkSession({connection: r })
// create the db, table and indexes to store sessions
sessionStore.setup()

var app = koa()
// used for cookie stuffs
app.keys = ['foo', 'bar']

app.use(session({
  store: sessionStore
})

Again, thanks for taking time to work on this! Just had an issue with a user who got a little confused because of the variable naming and thought others might have run into this. I might also be totally wrong about all this and missing something :). Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant