marpple / FxTS

A functional programming library for TypeScript/JavaScript
https://fxts.dev
Apache License 2.0
859 stars 64 forks source link

Incorrect data handling without toArray in pipeline using flatMap #279

Open HunSeol opened 3 weeks ago

HunSeol commented 3 weeks ago

Bug Report

💻 Code

const groups = [
  {
    options: [
      {
        id: 1,
        name: 'name-1',
        checked: true,
      },
      {
        id: 2,
        name: 'name-2',
        checked: false,
      },
    ],
  },
  {
    options: [
      {
        id: 3,
        name: 'name-3',
        checked: true,
      },
      {
        id: 4,
        name: 'name-4',
        checked: true,
      },
    ],
  },
  {
    options: [
      {
        id: 5,
        name: 'name-5',
        checked: false,
      },
    ],
  },
];

// Code that works as expected
const checkedOptionsWithArray = pipe(
  groups,
  flatMap((group) => group.options),
  filter((option) => option.checked),
  toArray
);
const optionIdsWithArray = pipe(
  checkedOptionsWithArray,
  map((option) => option.id),
  join('|')
);
const optionNamesWithArray = pipe(
  checkedOptionsWithArray,
  map((option) => option.name),
  join('|')
);

console.debug(optionIdsWithArray); // "1|3|4"
console.debug(optionNamesWithArray); // "name-1|name-3|name-4"

// Code that does not work as expected
const checkedOptionsWithoutArray = pipe(
  groups,
  flatMap((group) => group.options),
  filter((option) => option.checked)
);
const optionIdsWithoutArray = pipe(
  checkedOptionsWithoutArray,
  map((option) => option.id),
  join('|')
);
const optionNamesWithoutArray = pipe(
  checkedOptionsWithoutArray,
  map((option) => option.name),
  join('|')
);

console.debug(optionIdsWithoutArray); // "1|3|4"
console.debug(optionNamesWithoutArray); // "" --> empty string, This is problem.

🙁 Actual behavior

When toArray is not used after flatMap and filter, the intermediate values are removed, resulting in an empty string for optionNames.

🙂 Expected behavior

Both optionIds and optionNames should correctly display the joined string of IDs and names of checked options, regardless of the use of toArray.


Version Information

hg-pyun commented 2 weeks ago

@ppeeou It needs to determine if the case where toArray is not used constitutes a bug.

ppeeou commented 2 weeks ago

@HunSeol Thank you for reporting the issue.

This is not a bug.

Because you have consumed all of the Iterator in optionIdsWithoutArray In optionNamesWithoutArray , iterator.next() returns {done:true, value:undefined } .

If you want to check, change the execution order as follows.

const optionNamesWithoutArray = pipe(
 checkedOptionsWithoutArray,
 map((option) => option.name),
 join('|')
);

const optionIdsWithoutArray = pipe(
 checkedOptionsWithoutArray,
 map((option) => option.id),
 join('|')
);

This is something to be careful of when using the iterator object. To achieve the intended result, you must use fork, but it is still under development. https://github.com/marpple/FxTS/issues/70

To obtain the desired result with the current specification, you can use toArray to store the iterator result in memory.

HunSeol commented 2 weeks ago

be careful of when using the iterator object. you must use fork, but it is still under development.

Thank you for comment. I understood your intention.

To obtain the desired result with the current specification, you can use toArray to store the iterator result in memory.

Could you explain this comment more? How to store in memory using toArray?

HunSeol commented 2 weeks ago

I have one more question. As we know, Array.prototype.map is an immutable function because it does not modify the original array. So I think it makes us more confused. Do you have plans to add an immutable map function not fork function?

ppeeou commented 1 week ago

Could you explain this comment more? How to store in memory using toArray?

@HunSeol You were already using it :)

const checkedOptionsWithArray = pipe(
  groups,
  flatMap((group) => group.options),
  filter((option) => option.checked),
  toArray
);
const optionIdsWithArray = pipe(
  checkedOptionsWithArray,
  map((option) => option.id),
  join('|')
);
const optionNamesWithArray = pipe(
  checkedOptionsWithArray,
  map((option) => option.name),
  join('|')
);

I have one more question. As we know, Array.prototype.map is an immutable function because it does not modify the original array. So I think it makes us more confused. Do you have plans to add an immutable map function not fork function?

Not yet, but I'll think about it.