mapbox / mapbox/mapbox-gl-directions

Evaluate usage of Object.assign()

Open
#103 1 comment 0 reactions 0 assignees View on GitHub
auto-triaged bug
Dominant language
JavaScript
Stars
255
Forks
130
PR merge metrics
No merged PRs in 30d

Description

Currently in our reducer we use `Object.assign()` liberally. This works great for single-depth objects, and key value pairs where the `value` isn't another object. Working on #101 shows that if you pass the following options:

```
var directions = new MapboxDirections({
controls: {
inputs: false
}
});
```

The options reducer outputs the following object:

```
console.log(options); // { controls: { inputs: false } }
```

But I would expect the default `instructions` to still exist:

```
console.log(options); // { controls: { inputs: false, instructions: true } }
```

The source of the problem here is [`Object.assign()` here](https://github.com/mapbox/mapbox-gl-directions/blob/28936a8f86de94a5db82e0498356cf9feb093ffb/src/reducers/index.js#L45) where we add defaults and custom options, but deeply nested object values completely override instead of replacing individual key/value pairs. Example:

```javascript
var defaults = {
"hello": "world",
"items": {
"one": true,
"two": true
}
};
var custom = {
"hello": "earth",
"items": {
"one": false
}
};

var obj = Object.assign({}, defaults, custom);
console.log(obj); // =>
// {
// "hello": "earth",
// "items": {
// "one": false
// }
// }

// but I expect

// {
// "hello": "earth",
// "items": {
// "one": false,
// "two": true
// }
// }
```

We need to deep assign here: https://github.com/sindresorhus/deep-assign

cc @tmcw @springmeyer

Contributor guide

Open the contributing guide

Research direction

Start in src/reducers/index.js at the Object.assign() call linked in the issue and compare its behavior with the nested defaults example. Update the reducer's merge behavior so partial nested options retain unspecified defaults, then verify that controls.inputs: false still preserves controls.instructions: true.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.