staltz / cycle-onionify

MIGRATED! This was transfered to https://cycle.js.org/api/state.html
MIT License
279 stars 19 forks source link

List: repetative recreating of child components may be a problem #4

Open Hypnosphi opened 8 years ago

Hypnosphi commented 8 years ago

background: https://github.com/cyclejs/todomvc-cycle/commit/065c3048ee9d46b36fa071844486abb93622fa97#commitcomment-19533736

const taskSinks$ = array$.map(array =>
    array.map((item, i) => isolate(Task, i)(sources))

here, the Task component function is called again and again for each item whenever another item is added, removed or updated. So instead of usual workflow, where component or main function is used only for initial setup and then the stream library handles the changes, we run this setup over and over again. In fact, sources.onion.state$ inside the child component never emits twice, so it's no better then just plain value.

André suggested, that memoization could help here, and it definitely will, but it doesn't look possible without introducing ids

staltz commented 8 years ago

André suggested, that memoization could help here, and it definitely will, but it doesn't look possible without introducing ids

That's ok. IDs can be used as a way to opt-in for optimization. I view this like virtual DOM keys: in many cases you don't need them, but putting them in will make the underlying library do less guessing work.

staltz commented 7 years ago

I have some ideas for this, including one that would use diff and patch in order to know when to create a child component. I've done some experiments, but not enough to solve it.

So: work in progress.

kylecordes commented 7 years ago

Ugly, ad-hoc caching, in the meantime:

  const cache = new Map<number, any>();

  const childrenSinks$ = array$.map(array =>
    array.map((item, i) => {
      if (!cache.has(i)) {
        cache.set(i, isolate(InnerComponentHere, i)(sources));
      }
      return cache.get(i);
    })
  );

(I noticed a minor downside to creating components as functions rather than classes - each class is also its own type, suitable to use as the value type in the Map.)

staltz commented 7 years ago

@kylecordes check issue #28

kylecordes commented 7 years ago

@staltz Thank you; I'm now using it.