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

[@th.ing/transducer] weird behaviour when combining mapcat with multiplex #401

Closed
tien opened this issue Jul 26, 2023 · 4 comments
Closed

Comments

@tien
Copy link

tien commented Jul 26, 2023

const transducer = tx.multiplex(
  tx.comp(tx.mapcat((x) => x)),
  tx.map((x) => x)
);

tx.transduce(transducer, tx.push(), [[1, 2], [3]])

const expect = [[[1, 2], [1, 2]], [[3], [3]]]
const actual = [[[1, 2], [1, 2]], [3, [3]]] // array of size 1 got unexpectedly flattened

// Also TS definition can possibly improved
type ExpectType = [number[], number[]]
type ActualType = [number, number[]]
@postspectacular
Copy link
Member

Thank you - for multiplex() this is indeed unexpected behavior, but it's actually down to the expected & documented behavior of step() which is being used internally. To fix this for multiplex(), I think it's best to add a custom version of this stepper which doesn't have this special case handling (unwrapping) for single-value results. Will work on this today and release with next cycle...

@tien
Copy link
Author

tien commented Jul 26, 2023

Awesome, thanks for the quick response 💪

This lib is a god send btw 🙏

@postspectacular
Copy link
Member

Huston, we've got a problem... I've updated the code to disable unwrapping, but of course (in hindsight) all this has done is shifting the issue in the opposite way, i.e. in your above example from mapcat() to map():

const xf = tx.multiplex(tx.mapcat((x) => x), tx.map((x) => x));

// new version of step with unwrapping of single results disabled
// (you wont' be able to reproduce this on your end yet...)
const xs=tx.step(xf, false);

xs([1, 2]);
// [ [ 1, 2 ], [ [ 1, 2 ] ] ]

xs([3]);
// [ [ 3 ], [ [ 3 ] ] ]

As you can see, the issue now fixed for mapcat() but the results produced by map() also don't get unwrapped now and therefore have an additional level of nesting. However, I'm also not quite sure what to do about that (i.e. how to differentiate/detect when to unwrap and when not to...)

Will have to think about this some more...

@postspectacular
Copy link
Member

We could check if the result of a single item array with the item an array itself, but that's too fragile and will have a ton of edge cases. The only sure fire way would be to update the multiplex() args and allow disabling the current unwrap behavior (per given transducer), something like this:

multiplex(
  // provide as tuple to customize unwrap behavior
  [mapcat((x) => x), false],
  // or provide as before (with current unwrap behavior)
  map((x) => x)
)

IMHO that's the best/safest solution, also ensuring non-breaking behavior in existing userland code...

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

2 participants