thi-ng / umbrella

⛱ Broadly scoped ecosystem & mono-repository of 199 TypeScript projects (and ~180 examples) for general purpose, functional, data driven development
https://thi.ng
Apache License 2.0
3.35k stars 149 forks source link

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

Closed tien closed 1 year ago

tien commented 1 year ago
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 commented 1 year ago

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 commented 1 year ago

Awesome, thanks for the quick response 💪

This lib is a god send btw 🙏

postspectacular commented 1 year ago

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 commented 1 year ago

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...