Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Handle null options by not relying on object spread #1346

Open
nventuro opened this issue Dec 16, 2019 · 2 comments
Open

Handle null options by not relying on object spread #1346

nventuro opened this issue Dec 16, 2019 · 2 comments
Labels
topic:tech-debt Pending technical debt

Comments

@nventuro
Copy link
Contributor

A common pattern throughout the SDK when handling configuration is to merge default and received values using the spread operator. This works as long as unspecified keys are not present in the object, but fails if they exist with a null or undefined value. As a workaround, we use pickBy to remove these (but do so incorrectly, see #1231).

Having to remember these quirks increases the mental overhead when developing for the SDK: we should instead not rely on object spread and properly handle keys with nil values when applying defaults.

@frangio
Copy link
Contributor

frangio commented Dec 16, 2019

The alternative to using object spread is to define a function that merges an object with a set of default properties.

We should also enable --strictNullChecks in our TypeScript options, because it will help us catch scenarios where we're not handling undefined correctly.

@spalladino
Copy link
Contributor

Lodash#merge takes care of that. We should only watch out for array or object-like properties, in case we don't want the deep-merge recursive behaviour.

This method is like _.assign except that it recursively merges own and inherited enumerable string keyed properties of source objects into the destination object. Source properties that resolve to undefined are skipped if a destination value exists. Array and plain object properties are merged recursively. Other objects and value types are overridden by assignment. Source objects are applied from left to right. Subsequent sources overwrite property assignments of previous sources.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic:tech-debt Pending technical debt
Projects
None yet
Development

No branches or pull requests

3 participants