localvoid/ivi

stateful component opts/behaviors?

leeoniya opened this issue · 7 comments

hey @localvoid

looking at https://github.com/localvoid/ivi#components-1

since only the render function receives props, and the component closure only gets the instance, how would you pass some options to the component closure during component init?

for example VirtualizedList({chunkSize: 200}). or let's say your props were hoisted:

const VirtualizedList = component((c, opts) => {
  return (items) => htm`...`;
});

const MY_VIRT_OPTS = {chunkSize: 200};
VirtualizedList(MY_VIRT_OPTS, items);

is the recommendation to make a factory wrapper fn for VirtualizedList that accepts pre-defined opts?

function makeVirtualizedList(opts) {
  const VirtualizedList = component((c) => {
    return (items) => htm`...`;
  });

  return VirtualizedList;
}

const VirtualizedList200 = makeVirtualizedList({chunkSize: 200});

VirtualizedList200(items);

one example in domvm, shows the initial props also being passed to the closure, not just to render: https://github.com/domvm/domvm#hello-world

having these props at init time is often useful during construction. in the Stepper example above, it avoids having to refresh some _stepper within the closure on each render pass. this is nice when stepper is referentially consistent.

this patterm is not without its own footguns, however. (the framework has to be told that component instances should be matched up by their props as well, not just their factories). i guess this would be covered by getKey => stepper, but that gets expensive if getKey must always be fn.

EDIT: probably ignore this stepper example, it's better to just refresh a _stepper from the render fn.

This is a tricky one and I am still thinking about some nice API to deal with such use cases.

The main issues is that if we add initial props to an outer function, and capture them in some closure, it will lead to some form of a "memory leak" because entire closure frame is shared between different closures. React has a similar issue with their hooks API. When React component uses a hook without any dependencies (empty array), it will capture initial context frame for the entire lifespan of a component even if props aren't used by this hook, and captured by some adjacent hook.

In the worst case scenarios, this types of memory leaks will store initial app state for the entire duration of an application when it is a top-level component.

I guess there are two common use cases:

  • Static default value (can be solved with a factory function as you've describe above)
  • Default value passed as props
const DefaultValue = component((c) => {
  let _value;
  const onClick = () => {
    _value++;
    invalidate(c);
  }
  
  return (defaultValue) => {
    if (_value === void 0) {
      // Move all initialization from the outer scope here
      _value = defaultValue;
    }
    return 
  }
});

If it is a dependent value and it is used in some side effect, it should be passed to this side effect as an input parameter. And if it is a default value, it can be ignored in the areEqual hook. E.g.

https://github.com/localvoid/ivi-repl/blob/259cbc41f3ba91121373f0c97fce358f24df4d05/src/Editor.ts#L57

This is a type of problems that is so much easier to solve with Signal-like libraries, but again it just leads to different tradeoffs.

Maybe add a function like getProps(component) that will retrieve the current props from the component instance, and add a note in the documentation that there are some potential footguns with code like this:

const Example = component((c) => {
  const props = getProps(c);
  const fn = () => { /* captures props, creates memory leak */ };
  return () => {};
});

Added getProps() function that retrieves props value from component node.

Even if it is easy to create a memory leak with such API, in some use cases it makes code way much simpler, so I think that it is worth to have such function.

honestly, i'm not super convinced the leak profile is much different from the one created by refreshing props within the component closure via the render fn. the only difference is that the one created by passing props to the closure is opt-out rather than opt-in, which by itself could be a convincing argument not to do it. but...i think they're easily avoided by simply omitting the props arg from your userland component sig (and never referencing that arg), so even if ivi passes them to the closure, but they're never used/referenced, there's no leak.

i dont think this leaks, unless this fn then references props, (and is then itself used in the rendering fn template):

  const fn = () => { /* captures props, creates memory leak */ };

to me, these feel pretty equivalent from a leak risk:

const Example = component((c) => {
  let _props = getProps(c);

  const onclick = () => { console.log(_props.foo) };

  return (props) => {
    _props = props;

    return htm`<button @click={onclick}`>Click Me!</button>`;
  };
});
const Example = component((c, _props) => {
  const onclick = () => { console.log(_props.foo) };

  return (props) => {
    _props = props;

    return htm`<button @click={onclick}`>Click Me!</button>`;
  };
});

you can also plug the leak via props = null after you do what you need during init just ahead of returning the rendering fn. i think these leaks are O(1) anyways (they dont grow with component count).

i went back and forth on this in domvm and the biggest thing i didnt like was that your closured props "go stale" and have to be refreshed via render, but with React hooks normalizing stale closures 🤯 , this doesnt seem so crazy anymore.

i dont think this leaks, unless this fn then references props:

Yes, I just meant that if some closure "captures props", it creates some form of "memory leak". Even if it is a short-lived closure, it will stay alive for the entire duration of a component, because it is most likely some other closure is going to capture a component instance, or some other variable created in the outer scope.

to me, these feel pretty equivalent from a leak risk

Yes, they're quite similar, the only reason why I've chosen getProps() is that in some use cases it will be possible to write code like this without refreshing props:

const Example = component((c) => {
  const onclick = () => { console.log(getProps(c).foo) };

  return (props) => {
    return htm`<button @click={onclick}`>Click Me!</button>`;
  };
});

Also, component node is most likely to be captured by some closure, so with getProps() there will be slightly less memory consumption than capturing component+props. Yes, it is a super micro optimization that won't be even noticeable, but ivi is built around many different micro optimizations :)

i think these leaks are O(1) anyways (they dont grow with component count).

Yes, but I can easily imagine some use case with a top-level component that receives immutable initial app state and it is going to hold it for the entire duration of an app. And the problem is that sometimes it is quite hard to notice such "leaks", especially when minifiers are starting to inline functions.

👍 sounds good.