Conditional component creation

[lmatteis-wm]

Currently I have a simple Tree

class TemplateDemo extends lng.Application {
    static _template() {
        return {
            x: 20, y: 20,
            List: { type: ExampleList, visible: false }
        }
    }
    _init() {
        // let's generate dinamically some list items 
        // and give it to our list
        this.tag('List').items = [1,2,3,4].map((i) => ({label: i }))
    }
}

class ExampleList extends lng.Component {
    set items(items) {
        this.children = items.map((item, index) => {
            return {
                type: ExampleListItem,
                x: index * 70, //item width + 20px margin
                item //passing the item as an attribute
            }
        })
    }
}

Even though ExampleList visible property is set to false, the component is still executed from a CPU perspective. In fact all of its setters are called along with lifecycle methods. This also continues down the tree if any child components also have setters and so on. Causing lots of unnecessary CPU work because the component shouldn't even be visible. I understand visible is meant for rendering to GPU, but the CPU work mentioned still happens.

Ideally what I would like is to completely ignore running any of the code inside ExampleList given a specific property change:

static _template() {
    return {
        x: 20, y: 20,
        List:  ...hasItems ? { type: ExampleList } : {}
    }
}

Of course being static I cannot do this in the template.

Is something like this possible? I've tried updating .children = but many of my components are using bindProp and they break if I create the component outside _template.

For instance I tried doing something like this:

public set items(items) {
  // ever time items change, re-render children
  if (this.hasItems) {
    this.children = [ { type: ExampleList, foo: this.bindProp('foo') } ];
  } else {
    this.children = []
  }
}

But it breaks since it's not within a static _template.

Ideas?

[g-zachar]

Hi @lmatteis-wm,

As you pointed out, the visible flag is solely for the rendering purposes, therefore even though an element may not be visible, it still needs to be fully operational and initialised.

Currently, property bindings are quite limited in functionality and restricted to templates alone. They shouldn't be used for elements that are dynamically changing within the component because of the limitations you've noticed. I might look into the possibilities of extending bindProp functionality outside the template method.

Best regards

[lmatteis-wm]

Thanks @g-zachar ,

What is your suggestion on how to do this in general?

Imagine this template:

static _template () {
    return {
        MediaPlayer: { 
          type: MediaPlayer, 
          data: this.bindProp('data'),
          visible: this.bindProp('visible'),  
        },
    };
}

When MediaPlayer is instantiated it occurs some extra work on the CPU as well as when its props change (such as data). I would like for this work to only happen when the MediaPlayer is visible and hence only when the visible prop is true. An example would be great but I don't see a super easy way.

My solution was to have the template empty such as:

static _template () {
    return {
        MediaPlayer: {} // empty on purpose so MediaPlayer isn't instantiated
    }
}

And then I need to react to when both data and visible change and patch the tree accordingly? Such as:

set data(data) {
  this._data = data;
  if (this.visible) {
    // instantiate the MediaPlayer
    this.patch({
        MediaPlayer: { 
          type: MediaPlayer, 
          data: data,
          visible: this.visible,  
        },
    })
  } else {
    // remove it
    this.patch({
        MediaPlayer: undefined
    })
  }
}

// and also for visible
set visible(visible) {
  this._visible = visible;
  if (visible) {
    // instantiate the MediaPlayer
    this.patch({
        MediaPlayer: { 
          type: MediaPlayer, 
          data: this.data,
          visible: visible,  
        },
    })
  } else {
    // remove it
    this.patch({
        MediaPlayer: undefined
    })
  }
}

Is there an easier way to do this or do I have to define a setter for both data and visible and their private local values as well accordingly. Sounds like a pain but maybe I can abstract this work into a separate component. Ideas?

[g-zachar]

@lmatteis-wm

I would probably move instantiation of the MediaPlayer component somewhere in the app initialisation phase and reuse it whenever necessary if it causes stuttering. If you used visible property then you probably have similar architecture in place. If updating bound properties causes CPU spikes then I would control it using visibility flag as well, e.g:

static _template () {
    return {
        MediaPlayer: { 
          type: MediaPlayer, 
          data: this.bindProp('mpData'),
        },
    };
}

// Delegate setter to avoid unnecessary updates
set data(v) {
  if (this.visible) this.mpData = v;
}

[lmatteis-wm]

@g-zachar

I would probably move instantiation of the MediaPlayer component somewhere in the app initialisation phase and reuse it whenever necessary if it causes stuttering.

Interesting! How would I do that?

To be specific I'm dealing with "BaseTile" components that are rendered dynamically as the user scrolls more are fetched from server and rendered (using .childList.add() to add them) - think of a scrolling carousel made out of tiles.

Problem is that I'm noticing spikes in CPU whenever this happens and quite slow rendering times on old device. One avenue was just to reduce the BaseTile component code and avoid sub-components to render only if visible, and that's helping a bit, like mentioned in this thread.

But perhaps there's a more performant way to keep a single instance of the BaseTile component and being able to render that (with slightly different values) in a more performant way - avoiding sub-sequent instantiations? Not sure just thinking from what you said. How would that work?

Here's an example of the performance I'm seeing:

As you can see, this BaseTile component is just rendering for each single item in the carousel, in repetition.

Is there a way to be smart about this in Lightning performance-wise?

Thanks!

[g-zachar]

I would probably move instantiation of the MediaPlayer component somewhere in the app initialisation phase and reuse it whenever necessary if it causes stuttering.

Interesting! How would I do that?

That solely depends on your application. Most will have some kind of loading sequence in the beginning. Patching the root component using patch method in that period is the usual choice I suppose.

Problem is that I'm noticing spikes in CPU whenever this happens and quite slow rendering times on old device. One avenue was just to reduce the BaseTile component code and avoid sub-components to render only if visible, and that's helping a bit, like mentioned in this thread.

You can try splitting the batch into smaller chunks and create them sequentially in very small time intervals. You can also create template cards first and then update their textures (whether it's possible or not depends on the content of the cards).

Does the browser on your older device support web workers? This could also be the reason for performance. Do you experience the drop when you reuse same static texture for your components as well?

There are many factors individual to your case that make it very hard for me to be able to help effectively. Sometimes, for low end devices there is very limited space for improvement.

Best regards

[lmatteis-wm]

You can also create template cards first and then update their textures (whether it's possible or not depends on the content of the cards).

Do you mean to create and draw the components beforehand (say during app startup) and then patch their contents (say with the right image/text) afterwards? Would you have an example on how to do that?

Does the browser on your older device support web workers? This could also be the reason for performance. Do you experience the drop when you reuse same static texture for your components as well?

Yes it does although only 2 threads for concurrency.

Any ideas or "hacks" for how to get these tiles component rendered more smoothly and allow for less stutter would be great.

Also some things cannot go "upfront". For instance there's a search box that updates a list of these tile cards for every word entered. So that stutters.

Best,

[lmatteis-wm]

@g-zachar

Is there a way that I can patch a component and give it bindProps outside template?

  this.patch({
      Tile: { 
        type: Tile, 
        data: this.bindProp('data'),
        visible: this.bindProp('visible'),  
      },
  })

I know you mentioned this as a "feature" to implement binding outside template but was wondering if there's a workaround - maybe I can create an abstraction at the app level - because I don't think I can update the Lightning version anytime soon. And I've measured good performance improvement by conditionally instantiating components.

I know that bindProp() calls are converted to objects which then internally Lightning goes through to do the proper getter/setter stuff. In fact it calls Component.parseTemplate(). Maybe there's a way to call that method at this stage?

EDIT the closest I could get was this:

public _setup(): void {
    const instance = this.stage.c({
      type: this.elem
    });

    for (const propKey in this.props) {
      const propObj = this.props[propKey];
      this.parent.__bindProperty(propObj, instance, propKey);
    }

    this.parent?.childList.replace(instance, this);
.}

Problem is that calling '__bindProperty' in this moment (at _setup of a component) is too late as certain setters are called before the __bindProperty gets the chance to execute :( But I feel I'm almost there.

[g-zachar]

@lmatteis-wm

Other than implementing the binding method outside the template there is also the quite difficult problem of tracking existing bindings and clearing them to avoid memory leaks.

Property bindings are just syntax sugar built around setters with quite complicated implementation. Replacing / wrapping them with normal setter methods is going to be much easier and elegant solution in my opinion.

Best regards

[lmatteis-wm]

@g-zachar I created this abstraction which seems to work (inspired by how Element.__bindProperty is implemented):

Currently I have a component in my _template such as

{
    type: Labels,
    id: Tags.Labels,
    y: this.$labelsYPosition, // We use $dollarSign vars convention to identify bindProps
    w: this.$labelsWidth,
    titleText: this.$mainTitleText,
    titleDescriptionText: this.$titleDescriptionText,
    taglineText: this.$taglineText
}

But as mentioned I don't want to instantiate Labels immediately but only if a condition is true. So I thought to wrap the Labels into a "proxy component" I call ConditionalRendering:

{
    type: ConditionalRendering, // <-- Instead of "Labels"
    elem: Labels, // <-- Pass the component as prop so we can instantiate later
    cond: this.$shouldShowLabels, // <-- "cond" is the prop that decides whether to instantiate the component or not
    id: Tags.Labels,
    y: this.$labelsYPosition, 
    w: this.$labelsWidth,
    titleText: this.$mainTitleText,
    titleDescriptionText: this.$titleDescriptionText,
    taglineText: this.$taglineText,
    // Here I pass the bindProp objects again so I know which are the ones I need to bind against the later created component
    props: {
      y: this.$labelsYPosition, 
      w: this.$labelsWidth,
      titleText: this.$mainTitleText,
      titleDescriptionText: this.$titleDescriptionText,
      taglineText: this.$taglineText,
    }
}

And inside ConditionalRendering I have something like (a bit ugly still):

set cond(cond) {
  this._cond = cond;
  // Run only after _setup
  if (this._hasSetupRun) {
    this._tryAndCreateInstance();
  }
}
_setup() {
  this._tryAndCreateInstance();
  this._hasSetupRun = true;
}

_tryAndCreateInstance() {
  if (this.cond && !this._hasInstance) {
      const instanceObj = {
        type: this.elem,
        ref: this.ref
      };

      // Copy all bindProps to current constructor instance
      for (const propKey in this.props) {
        instanceObj[propKey] = this[propKey];
      }

      // Create instance
      const targetObj = this.stage.c(instanceObj);

      // Bind all the props so everything is reactive
      for (const propKey in this.props) {
        const propObj = this.props[propKey];
        const targetProp = propKey;

        const obj = targetObj;
        const prop = targetProp;
        const propName = propObj.__name;
        const func = propObj.__func ? propObj.__func : (context) => context[propName];

        if (!this.hasOwnProperty(propName)) {
          this[`__prop_bindings_${propName}`] = [{ __obj: obj, __prop: prop, __func: func }];
          Object.defineProperty(this, propName, {
            set: (value) => {
              this[`__prop_${propName}`] = value;
              for (const { __obj, __prop, __func } of this[`__prop_bindings_${propName}`]) {
                __obj[__prop] = __func(this);
              }
            },
            get: () => this[`__prop_${propName}`]
          });
        } else {
          if (this[`__prop_bindings_${propName}`]) {
            this[`__prop_bindings_${propName}`].push({ __obj: obj, __prop: prop, __func: func });
          } else {
            // propName already exists (coming from top) but there's no bound setter to it
            this[`__prop_bindings_${propName}`] = [{ __obj: obj, __prop: prop, __func: func }];
            Object.defineProperty(this, propName, {
              set: (value) => {
                this[`__prop_${propName}`] = value;
                for (const { __obj, __prop, __func } of this[`__prop_bindings_${propName}`]) {
                  __obj[__prop] = __func(this);
                }
              },
              get: () => this[`__prop_${propName}`]
            });
          }
        }
      }

      // Replace the current `ConditionalRendering` proxy component with this newly created one
      this.parent?.childList.replace(targetObj, this);
      this._hasInstance = true;
    }
  }
}

Thoughts? Do you see any problems with this approach?

[g-zachar]

@lmatteis-wm

I wouldn't recommend this approach because of maintainability reasons. Other than that, you still have to make sure the component will release its bindings once its child elements are removed from it, as it is now it will leak memory.

Best regards

[lmatteis]

@g-zachar makes sense. You can mark this as closed. Thanks for feedback.

[lmatteis-wm]

@g-zachar would it makes sense to reopen this in the idea to create a bindProp as a utility method that can be used on top of lightning? I do see potential in having a set of app-level utilities that can be used without bloating the core.

This issue of having large invisible sub-components where the setter chains is executed and instances created even if not visible is popping up again for us on lower-end devices causing unnecessary work.

Would be nice to use this space to brainstorm on a bindProp utility method that can be used outside _template methods.