feat(data-point/reducer-default: Add the ReducerDefault type #194
Conversation
| ### <a name="reducer-default">withDefault</a> | ||
|
|
||
| The **withDefault** reducer adds a default value to any reducer type. If the reducer resolves to `null`, `undefined`, `NaN`, or `''`, | ||
| the default is returned instead. |
There was a problem hiding this comment.
The default value overwrites null, undefined, NaN, '' (but it does not overwrite 0 or false). If anybody has opinions about this I'm open to changing it.
| reducer[REDUCER_SYMBOL] = true | ||
| reducer[IS_REDUCER] = true | ||
| if (_.has(options, 'default')) { | ||
| reducer[DEFAULT_VALUE] = { value: options.default } |
There was a problem hiding this comment.
because the default is wrapped in this object, the hasDefault function still returns true if the default value is falsy
| if (hasDefault(reducer)) { | ||
| const _default = reducer[DEFAULT_VALUE].value | ||
| const afterResolve = reducers.ReducerDefault.resolve | ||
| return resolve.then(acc => afterResolve(acc, _default)) |
There was a problem hiding this comment.
I thought about using the default when a reducer throws an error:
// this instead of the current line 64
return resolve
.catch(e => ({ value: null }))
.then(acc => afterResolve(acc, _default))Any thoughts on doing this? It could even be a setting that's passed to withDefault:
const throwError = () => { throw new Error() }
const reducer = withDefault(throwError, 'DEFAULT_VALUE', { catch: true })
// or just... withDefault(throwError, 'DEFAULT_VALUE', true)
dataPoint.resolve(reducer, {}) // => 'DEFAULT_VALUE'
| const reducerType = reducers[reducer.type] | ||
| if (!reducerType) { | ||
| throw new Error(`Reducer type '${reducer.type}' was not recognized`) | ||
| let resolve = getResolveFunction(reducer) |
There was a problem hiding this comment.
why are you overwriting resolve here? above line is also setting it, any special reason?
There was a problem hiding this comment.
overwriting it just because it's one less variable name to think up
| */ | ||
| function resolve (accumulator, _default) { | ||
| let value = accumulator.value | ||
| if (Number.isNaN(value) || isNil(value) || value === '') { |
There was a problem hiding this comment.
looking at isNil in lodash I dont think its worth depending on it, + its documentation might not be what it actually is
link above is TOO SLOW!!!
/**
* Checks if `value` is `null` or `undefined`.
*
* @static
* @memberOf _
* @since 4.0.0
* @category Lang
* @param {*} value The value to check.
* @returns {boolean} Returns `true` if `value` is nullish, else `false`.
* @example
*
* _.isNil(null);
* // => true
*
* _.isNil(void 0);
* // => true
*
* _.isNil(NaN);
* // => false
*/
function isNil(value) {
return value == null;
}I would recommend instead do a manual check against undefined and null
There was a problem hiding this comment.
done (I also looked up the lodash source before using isNil though 😊)
also removes the use of _.isNil
What: Adds the
ReducerDefaulttype (closes #169)Why: This will add default values to any reducer.
How: Instead of actually creating a new reducer type, this just adds a
defaultproperty to any existing reducer. The property uses a symbol to avoid any future conflicts.Checklist: