Cancel args memorization if func throws exception

[zandroid]

The issue is when func throws exception the last args already saved and next calling of memoized func will return null instead of recalculating func again. If we save args after calling of func we save args only for "good" result and don't for exceptions.

[coveralls]

Coverage remained the same at 100.0% when pulling 1c106a8 on zandroid:rethrow-exception into 9dd61bf on reactjs:master.

[codecov-io]

Current coverage is 100%

Merging #144 into master will not change coverage

@@           master   #144   diff @@
====================================
  Files           1      1          
  Lines          13     13          
  Methods         0      0          
  Messages        0      0          
  Branches        0      0          
====================================
  Hits           13     13          
  Misses          0      0          
  Partials        0      0          

Powered by Codecov. Last updated by 9dd61bf...28873e8

[ellbee]

Hey @zandroid, would it be possible to add a test case that fails on the current version and passes with your fix please?

[zandroid]

@ellbee, sure, have added two tests.

[coveralls]

Coverage remained the same at 100.0% when pulling 28873e8 on zandroid:rethrow-exception into 9dd61bf on reactjs:master.

[ellbee]

Thanks @zandroid.

I've checked the test out, but I'm not sure I fully understand the problem this is trying to fix. Is it that it doesn't throw an exception the second time it receives the same set of arguments consecutively? What problem does that cause you?

You mention above the return value being null, but that is only returned when the selectors previous result was null. So either it never calculated a result without throwing (returning the default value of null) or the previous result was actually null.

[zandroid]

it doesn't throw an exception the second time it receives the same set of arguments

It's exactly the issue that I have in my code. But anyway I think selector should not memoize (default or previous) result for current args when func throws exception.

There may be other issue, not for default null value, for example:

const selector = createSelector(state => state.a, (a) {
  if (a > 1) throw new Error()
  return a;
})
selector({ a: 1 }) // => 1 - OK
try {
  selector({ a: 2 }) // => throw error - As expected
} catch (e) {}
selector({ a: 2 }) // => 1 - What?!

[ellbee]

Ok, thanks! So the problem is that it doesn't throw the second time—the return value being null is not relevant. It seems strange to have your selectors throwing errors, but I think I agree that the behaviour in this PR makes more sense.

[ellbee]

@zandroid Released in v2.5.2. Thanks for the PR!

[zandroid]

A lot of thanks. I agree that throwing exceptions from selector is not usual case, but we use it in our selectors to detect incorrect id of requested entity and show 404, 403 or 401 pages.