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

Fixed Bug #19

Merged
merged 6 commits into from
Jan 28, 2017
Merged

Fixed Bug #19

merged 6 commits into from
Jan 28, 2017

Conversation

shhdgit
Copy link

@shhdgit shhdgit commented Jan 19, 2017

There some different between babel version and node6 version, that make node6 version set a new session every time.

babel:

    await next();

    // if not changed
    if(old == JSON.stringify(ctx.session)) return;   ** finish **

    // clear old session if exists
    if(id) {
        await opts.store.destroy(id);
        id = null;
    }

    // set new session
    if(ctx.session && Object.keys(ctx.session).length) {
        let sid = await opts.store.set(ctx.session, Object.assign({}, opts, {sid: id}));
        ctx.cookies.set(opts.key, sid, opts);
    }

node6:

    promise.then(() => {
        old = JSON.stringify(ctx.session);

        return next();
    }).then(() => {
        // no modify
        if(old == JSON.stringify(ctx.session)) return;

        // destory old session
        if(id) return opts.store.destroy(id);
        
    }).then(() => {                             ** always through **
        // clear id
        id = null;

        if(ctx.session && Object.keys(ctx.session).length) {
            // set new session
            return opts.store.set(ctx.session, Object.assign({}, opts, {sid: id})).then(sid => {
                ctx.cookies.set(opts.key, sid, opts)
            });
        }
    });

@codecov-io
Copy link

Current coverage is 91.83% (diff: 100%)

Merging #19 into node6 will not change coverage

@@              node6        #19   diff @@
==========================================
  Files             2          2          
  Lines            49         49          
  Methods           7          7          
  Messages          0          0          
  Branches         10         10          
==========================================
  Hits             45         45          
  Misses            4          4          
  Partials          0          0          

Powered by Codecov. Last update af1986a...ad5f841

@Secbone
Copy link
Owner

Secbone commented Jan 20, 2017

@shhdgit I got the problem, but did you have any other way to fix it than throwing an error?

@shhdgit
Copy link
Author

shhdgit commented Jan 20, 2017

@Secbone xieranmaya/blog#5
这篇文章能给一些灵感?优雅一点,也复杂一点。

@Secbone Secbone merged commit 710fa46 into Secbone:node6 Jan 28, 2017
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 this pull request may close these issues.

3 participants