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

[Performance Advice] Patching to single element or custom elements #430

Open
hassanzohdy opened this issue Oct 10, 2019 · 16 comments
Open

Comments

@hassanzohdy
Copy link
Contributor

Hello,

I'm using IDOM to manipulate my HTML DOM which works perfectly fine.

Here is a code snippet to illustrate my point

The main page The static one

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <meta http-equiv="X-UA-Compatible" content="ie=edge">
    <title>Some Title</title>
</head>
<body>
    <div id="app"></div>
</body>
</html>

Now when the user hits the /home page, the #app gets the following component content:

home-page.component.html

<div class="container">
    <h1>Welcome Home</h1>
    <hello-world-form></hello-world-form>
</div>

hello-world.component.html

<form (submit)="this.submitForm($el)">
    <input type="text" name="username" [value]="this.username" />
</form>

I'm using Two-way data binding which is applied in the username input as when the this.username property is changed, the input value is changed as well and vice versa.

When any change happens to any property i.e this.username i re-run the patcher from the home-page.component.html as it will go through all elements from the top.

the <hello-world> component is removed and its content is added instead.

For example the actual output in the browser will be something like:

<div class="container">
    <h1>Welcome Home</h1>
    <form (submit)="this.submitForm($el)">
        <input type="text" name="username" [value]="this.username" />
    </form>
</div>

Now my question: As i mentioned earlier, i'm applying the patch function to my #app tag which contains the content of the home-page.component.html file.

Should i leave the hello-world tag in the dom and apply the patch function to that element? i.e the dom will look like:

<div class="container">
    <h1>Welcome Home</h1>
    <hello-world>        
        <form (submit)="this.submitForm($el)">
            <input type="text" name="username" [value]="this.username" />
        </form>
    </hello-world>
</div>

Or leaving the patch function to the #app element.

@hassanzohdy
Copy link
Contributor Author

@sparhami Any notes?

@iteriani
Copy link
Collaborator

iteriani commented Nov 22, 2019

Within Google we do something similar to keeping a element. If you inspect some google properties like drive or playstore, there are a sprinkling of c-wiz tags that represent patch boundaries.

However, the extra tag does carry around some bloat that our users don't like, so our compiler statically drops in something like

<form is="hello-world">...</form>

and we do a patchouter from there (when we know it is exactly one element).

Keep mind that some of our smaller apps just rerender from #app anyway (when the store changes or something)

@hassanzohdy
Copy link
Contributor Author

@iteriani thanks for reply, but when should i implement the approach of patching to the wrapping custom element, or let's say when i say that the application is getting bigger from being rendered from the root element?

For example i'm using IDOM with mentoor.io which sometimes contains like thousands of elements, which can any of it be modified and or added/deleted in any time , when that happens, i patch the root element and the cycle starts from the top again.

That's why i posted this topic, from a UX the performance is pretty much perfect in all devices that use the website either mobiles or desktops, but as a matter of improvement i was wondering if i should go with rendering the custom tags inside the dom.

@iteriani
Copy link
Collaborator

From a framework perspective, we had to implement patching of single elements so we could embed lifecycle hooks and skipping mechanics. Our users also liked the ability to develop and test components in isolation, which means that every component should be able to repatch itself when their own data changes.

If performance seems fine for now, it's probably not a big deal to keep repatching from the top. However, there are devex reasons (and potential perf reasons if you plan on scaling to large eramounts of dom) to having finer grained patches

@lilactown
Copy link

lilactown commented Nov 26, 2019

@iteriani can you explain a bit more how you patch in a child template using patchOuter? I'm trying to figure this out, where I have something akin to:

function templateB(el) {
  id.patch(el, () => id.text("goodbye"));
}

function templateA(el) {
  id.patch(el, () => {
    id.elementOpen("div")
    id.text("hello")
    templateB(id.currentElement());
    id.elementClose("div");
  });
}

Right now, this blows out the text "hello" with "goodbye", whereas I'd like to have both rendered within the parent div.

@iteriani
Copy link
Collaborator

iteriani commented Nov 26, 2019

The output of our templates emits something like

function templateB() {
  id.text("goodbye");
}

function templateA() {
    id.elementOpen("div")
    id.text("hello")
    templateB();
    id.elementClose("div");
}

and we use something like

id.patch(el, templateA)

@lilactown
Copy link

How does that solve the above problem you were talking about, where the child template might need to patch in new changes based on some bound data changing?

@iteriani
Copy link
Collaborator

We have component classes that record their own input parameters as well as internal state. When that changes, we can patch on a subtree.

https://github.com/google/closure-templates/blob/master/javascript/element_lib_idom.ts#L62

Similarly, having the templates like above mean that we can take a given element and execute the method at that pointer.

@lilactown
Copy link

OK, trying to understand this from scratch so I'm not very educated on the internals of closure-templates / Soy.

It looks like SoyElement instances are assigned a node when they are initially rendered; how does this assignment happen? How do subsequent re-renders of a template get assigned a new node, and still patch the old DOM?

I appreciate you taking the time to respond to my newbie questions!

@iteriani
Copy link
Collaborator

You can take a look here

https://github.com/google/closure-templates/blob/7132b6ac3001718b31891f61c584772e55d54f69/javascript/soyutils_idom.ts#L81

when we expect a SoyElement to be created on a root element, we call this to look for an element with the expected key. If it does not exist, create one and continue the patch from the soy element.

@hassanzohdy
Copy link
Contributor Author

@iteriani
Based on your last two replies, i can do perform capturing current location of a component but still can not figure how to patch it only on some certain changes happens inside it.

Consider the following:

// main function

function page() {
    id.elementOpen('div', null, null, 'id', 'main-div');
        
        id.elementOpen('span');
            id.text('Spanner');
        id.elementClose('span');

        callComponentA();
        
        id.elementOpen('span');
            id.text('Another Spanner');
        id.elementClose('span');

    id.elementClose('div');
}

// ComponentA 
class ComponentA {
    constructor() {
        this.nameIsChanged = false;

        // for sake of example
        setTimeout(() => {
            this.nameIsChanged = true;
            // re render again in somehow...
        }, 5000);
    }
    render() {
        id.elementOpen('div');
        id.text('Hello');

        if (this.nameIsChanged) {
            id.text('You Changed Your Name!');
        } 

        id.elementClose('div');
    }
}

// patch the page
id.patch(document.getElementById('app'), page);

Here in that example above, can we perform patch over the ComponentA without patching the entire page?

In other words, how would we update the position of the callComponentA, i can't think of any solution but to create a wrapper tag Which` i'm trying to avoid.

Also adding a wrapper tag for each component, won't that affect the overall performance by adding new unnecessary nodes to the DOM?

Aren't there a similar way to React Fragment that wraps up all its element into a some mystery holder?

Sorry for the too many questions but this is really driving me crazy.

@iteriani
Copy link
Collaborator

iteriani commented Jan 2, 2020

Sorry just got back to this! The way we do this should be documented in these files

https://github.com/google/closure-templates/blob/master/javascript/element_lib_idom.ts
https://github.com/google/closure-templates/blob/master/javascript/soyutils_idom.ts#L81

one example output of "callComponentA" is in this snippet

/**
 * @param {!incrementaldomlib.IncrementalDomRenderer} idomRenderer
 * @param {null=} opt_data
 * @param {?goog.soy.IjData=} opt_ijData
 * @return {void}
 * @suppress {checkTypes}
 */
var $simpleState = function(idomRenderer, opt_data, opt_ijData) {
  soyIdom.$$handleSoyElement(idomRenderer, $SimpleStateElement, xid('ns.simpleState-0'), opt_data, opt_ijData);
};
exports.simpleState = $simpleState;
if (goog.DEBUG) {
  $simpleState.soyTemplateName = 'ns.simpleState';
}
$simpleState.contentKind = SanitizedContentKind.HTML;

/** @interface */
var $SimpleStateElementInterface = class {
};
exports.SimpleStateElementInterface = $SimpleStateElementInterface;
/**
 * @extends {soyIdom.$SoyElement<null,!$SimpleStateElementInterface>}
 * @implements {$SimpleStateElementInterface}
 */
var $SimpleStateElement = class extends soyIdom.$SoyElement {
  /**
   * @param {null} opt_data
   * @param {!goog.soy.IjData=} opt_ijData
   */
  constructor(opt_data, opt_ijData) {
    super(opt_data, opt_ijData);
    /** @private {number} */
    this.state_s = 3;
  }

  /**
   * @param {!incrementaldomlib.IncrementalDomRenderer} idomRenderer
   * @param {null} opt_data
   * @protected
   * @override
   * @suppress {checkTypes}
   */
  renderInternal(idomRenderer, opt_data) {
    var opt_ijData = this.ijData;
    if (idomRenderer.maybeSkip(idomRenderer, idomRenderer.open('div', xid('ns.simpleState-0')))) {
      return true;
    }
    if (goog.DEBUG && soy.$$debugSoyTemplateInfo) {
      idomRenderer.attr('data-debug-soy', 'ns.simpleState third_party/java_src/soy/integrationtest/incrementaldomsrc/simple_state_vars_examples.soy:17');
    }
    idomRenderer.applyAttrs();
    idomRenderer.open('span', xid('ns.simpleState-1'));
    idomRenderer.applyAttrs();
    soyIdom.$$print(idomRenderer, this.state_s);
    idomRenderer.text(' is the correct answer!');
    idomRenderer.close();
    idomRenderer.close();
    return false;
  }

  /** @return {number} */
  getS() {
    return this.state_s;
  }

  /**
   * @param {number} s
   * @return {!$SimpleStateElement}
   */
  setS(s) {
    soy.asserts.assertType(typeof s === 'number', 's', s, 'number');
    this.state_s = s;
    return this;
  }
};

@hassanzohdy
Copy link
Contributor Author

I read these files but it doesn't illustrate how that would happen in the given case above.

Also the snippet is not so clear to get the point from it.

Can you please provide a snippet code using IDOM without Soy to be more clear to me.

Also an idea came into mind that when the inner component has change i will trigger update from its parent component/element which will re-render the entire parent component "the component to be updated plus any other sibling component/element (s)".

Would that be a proper approach to go with?

@sgammon
Copy link

sgammon commented Jan 28, 2020

@iteriani thank you for sharing that code snippet, that's actually very helpful for use cases that do include soy.

how is this code generated? is it coming from the soy compiler, or somewhere else? just wondering because we are also looking into how we might componentize.

@iteriani
Copy link
Collaborator

This is part of the Soy compiler itself. Instead of writing {template}, you write {element}. So something like

{element .foo}
{@State initCount := 0}
{$initCount}
{/element}

@sgammon
Copy link

sgammon commented Jan 29, 2020

@iteriani wtf that's amazing

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

4 participants