Add resave option #47
Conversation
|
Thanks @interpretor, another great addition (: We will review the PR shortly and hopefully be able to land the feature then. |
| event.res.on('finish', async () => { | ||
| // Session id may not exist if session was deleted | ||
| const session = await getSession(event) | ||
| if (!session) { | ||
| return | ||
| } | ||
|
|
||
| await setStorageSession(session.id, event.context.session) | ||
| if (sessionOptions.resave || !equal(event.context.session, source)) { |
There was a problem hiding this comment.
There was a problem hiding this comment.
Hey @interpretor, here you suggest to merge #48 and #41 (and abandon current one #47), while in #48 (comment) you suggest to merge #36, #41 and #47.
Can you please bring your reasoning on those changes and suggest from which one should I start a review? Perhaps #36 or #41?
There was a problem hiding this comment.
Hey @valiafetisov, I think you misunderstood me. Here I suggest to merge #48, as it is based on #36, #41 and #47. #36 is already merged, and I don't recommend to merge #41 or #47 individually. Instead, I recommend to merge only #48 as it does handle the options from #41 and #47 better.
So it would be best to start the review from #48.
Forces the session to be saved back to the session store, even if the session was never modified during the request. This is the current default behavior, with the
resaveoption it is possible to store only changes to the session.This is also an option of
express-sessionto reduce the risk of race conditions where a client makes two parallel requests and changes made to the session in one request may get overwritten when the other request ends, even if it made no changes.