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

Incorrect data handling without toArray in pipeline using flatMap #279

Open
HunSeol opened this issue Jun 19, 2024 · 5 comments
Open

Incorrect data handling without toArray in pipeline using flatMap #279

HunSeol opened this issue Jun 19, 2024 · 5 comments
Assignees

Comments

@HunSeol
Copy link

HunSeol commented Jun 19, 2024

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 optionIds: "1|3|4"
  • Expected 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.

  • Expected optionIds: "1|3|4"
  • Expected optionNames: "name-1|name-3|name-4"

Version Information

  • Operating system: [Your OS]
  • Typescript: 4.9.5
  • Nodejs: 20.12.0
@hg-pyun hg-pyun added the bug Something isn't working label Jun 23, 2024
@hg-pyun hg-pyun removed the bug Something isn't working label Jun 24, 2024
@hg-pyun
Copy link
Collaborator

hg-pyun commented Jun 24, 2024

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

@ppeeou
Copy link
Member

ppeeou commented Jun 25, 2024

@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.
#70

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

@HunSeol
Copy link
Author

HunSeol commented Jun 26, 2024

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
Copy link
Author

HunSeol commented Jun 26, 2024

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
Copy link
Member

ppeeou commented Jul 1, 2024

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.

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

No branches or pull requests

3 participants