feat: support promise api

[lokshunhung]

Reolves #24: This PR adds promise support for FastifyRequest#destroySession and the get, set, destroy methods on FastifyInstance#sessionStore.

The session store methods are wrapped with a new function to inspect whether a callback argument is passed, and will return a promise if the callback is missing. This approach does not mutate/monkey-patch methods on the store instance to avoid breaking anything. Tests for this are included here.

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[mcollina]

I'm sorry but this is not what the issue is about. We would like to support stores written with async functions, not creating a proxy.

[lokshunhung]

@mcollina Thanks for taking a look, unfortunately i have misunderstood the issue. Is there any existing promise-based store packages that the library would like to support? I could use it as reference and work on it. Cheers

[mcollina]

@SimenB wdyt? Do you know anything?

[SimenB]

What I originally requested was a way to use promises instead of callbacks in the API exposed by @fastify/session. Since this module is made for compat with Express' stores, I don't think any of those (pluggable stores) being promise based is anywhere close to happening.

So what I wanted is essentially what this PR has implemented - wrap any callback APIs with promises.

(as mentioned in the issue, request.destroySession is also a callback, but all other methods today are from the stores which are plugged in)

[lokshunhung]

@mcollina I can continue to work on it if you think this could be something to be added to the library. Currently the CI fails due to coverage which should not be too hard to fix.

[mcollina]

Great news! Could you fix the remaining tests @lokshunhung ?

[lokshunhung]

Sure, I can take a look next morning. It's 12am now so good night guys.

[coveralls]

Pull Request Test Coverage Report for Build 1428409433

• 39 of 39 (100.0%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 1420719549:	0.0%
Covered Lines:	223
Relevant Lines:	223

---

💛 - Coveralls

[lokshunhung]

@mcollina I fixed the tests and updated the docs. It's ready now.

[SimenB]

late suggestion for alternative approach, but current code works for my use cases 🙂

[lokshunhung]

If the comment from @SimenB is resolved, I think this can be merged

[SimenB]

this works fine for me as is (it adds what I originally requested in a nice and clean way), but I do think the API consistency should be solved first (to avoid potential churn)

[zekth]

Lgtm. Good work

[mcollina]

I do think the API consistency should be solved first (to avoid potential churn)

Could you clarify what you mean by "first"?

[SimenB]

I do think the API consistency should be solved first (to avoid potential churn)

Could you clarify what you mean by "first"?

Before landing this PR, as I think a unified API will remove the reason to promisify sessionStore in the first place (thus change this PR quite fundamentally)

[mcollina]

@SimenB so you prefer if #34 was solved first and then this to be reworked on top. Great.

[SimenB]

Yup!