jaredhanson / connect-flash

Flash message middleware for Connect and Express.
MIT License
1.24k stars 158 forks source link

Getting a flash message also clears it #7

Closed blakehaswell closed 11 years ago

blakehaswell commented 11 years ago

It is expected that methods which return data should not have side effects. Currently, if I get data from the flash then that data is cleared:

req.flash('url');
// [ '/users/1']

req.flash('url');
// []

This leads to confusing code:

if (req.flash('url') && req.flash('url').length) {
    // Never gets executed.
    req.flash('url', req.flash('url'));
}

To get around this problem I need to add unnecessary temps to my methods and, worse, unnecessary properties to my methods.

There should be an explicit method to clear items from the flash, and getting them should not have side effects.

jaredhanson commented 11 years ago

This is the long standing convention of the flash, as set by rails and implemented in express 2.x

It may be odd, but that is the intended behavior

Sent from my iPhone

On Aug 13, 2013, at 6:51 PM, Blake Haswell notifications@github.com wrote:

It is expected that methods which return data should not have side effects. Currently, if I get data from the flash then that data is cleared:

req.flash('url'); // [ '/users/1']

req.flash('url'); // [] This leads to confusing code:

if (req.flash('url') && req.flash('url').length) { // Never gets executed. req.flash('url', req.flash('url')); } To get around this problem I need to add unnecessary temps to my methods and, worse, unnecessary properties to my methods.

There should be an explicit method to clear items from the flash, and getting them should not have side effects.

— Reply to this email directly or view it on GitHub.

blakehaswell commented 11 years ago

Thanks for the quick response. I understand that the behaviour is intentional, but the convention (however long standing) is bizarre. Any idea why this convention exists?

I have situations where I need to access the same value from the flash for different bits of logic, and now I need to pass arguments around when they should be unnecessary. :pensive:

jaredhanson commented 11 years ago

Nope. If this doesn't fit your use case, don't use it.

blakehaswell commented 11 years ago

Apologies, I have only just realised how flash is intended to be used. My experience with flash has been with Java’s Play! Framework, where the flash only exists for a single request.

Because I had that expectation I was trying to maintain the flash between requests when that was completely unnecessary. Thanks again for your quick responses.

jaredhanson commented 11 years ago

Glad you figured it out!