Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'path' parameter to the mapper function #48

Closed
wants to merge 4 commits into from

Conversation

rphippswiley
Copy link

@rphippswiley rphippswiley commented Aug 16, 2023

Hi @sindresorhus ,

I'm completing the abandoned PR : #43 , thank you @leonardoraele for doing most of the work.

Adding a path parameter to the mapper function, which contains the list of keys to reach the current value from the source object. This is useful when deep: true to know where you are in the map process. The path parameter is undefined an empty array if deep option is false (the default).

I'd love to have the path available in the mapper, too. I've hopefully address your comments in the original PR, I'll outline them in this PR.

index.d.ts Outdated
@@ -85,6 +86,9 @@ const newObject = mapObject({FOO: true, bAr: {bAz: true}}, (key, value) => [key.

const newObject = mapObject({one: 1, two: 2}, (key, value) => value === 1 ? [key, value] : mapObjectSkip);
//=> {one: 1}

const newObject = mapObject({foo: {bar: [2], baz: [1, 2, 3]}}, (key, value, source, path) => path.join('.') === 'foo.baz' ? [key, 3] : [key, value], {deep: true});
Copy link
Author

@rphippswiley rphippswiley Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


For arrays, the key is the index of the element being mapped.

```js
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if this simplified example is ok, in response to https://github.com/sindresorhus/map-obj/pull/43/files#r884235030

test.js Show resolved Hide resolved
@rphippswiley rphippswiley marked this pull request as ready for review August 16, 2023 16:12
@leonardoraele
Copy link

@sindresorhus

readme.md Outdated Show resolved Hide resolved

console.log(result);
//=> {foo: {bar:[2], baz: [3, 3, 3]}}
```
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readme and index.d.ts should be in sync.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took this to mean you want the path annotated in the definition file, let me know if I misunderstood.

readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved

console.log(result);
//=> {foo: {bar:[2], baz: [3, 3, 3]}}
*/
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect indent.

Comment on lines +29 to +30
@example
import mapObject from 'map-obj';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@example
import mapObject from 'map-obj';
@example
import mapObject from 'map-obj';

/**
When using `deep: true`, this is the sequence of keys to reach the current value from the `source`, otherwise it is an empty array.
For arrays, the key is the index of the element being mapped.
@example
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example needs to be wrapped in a codeblock.

import mapObject from 'map-obj';

const object = {foo: {bar: [2], baz: [1, 2, 3]}}
const mapper = (key, value, source, path) => path.join(".") === "foo.baz" ? [key, 3] : [key, value];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single-quotes, as commented earlier.

const result = mapObject(object, mapper, {deep: true});

console.log(result);
//=> {foo: {bar:[2], baz: [3, 3, 3]}}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//=> {foo: {bar:[2], baz: [3, 3, 3]}}
//=> {foo: {bar: [2], baz: [3, 3, 3]}}

);
});

test('mapper argument `path` contains the sequence of keys to reach the current value from the source', t => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs t.plan too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants