Polymer / pwa-starter-kit

Starter templates for building full-featured Progressive Web Apps from web components.
https://pwa-starter-kit.polymer-project.org
2.36k stars 431 forks source link

Why are elements not loaded dynamically in the kit? #265

Closed freshgrapes closed 5 years ago

freshgrapes commented 5 years ago

Why are elements in the pwa-starter-kit loaded like this <shop-item></shop-item> instead of a more dynamic approach like <div id="xxx"></div> and later on insert the element <shop-item> in JavaScript using appendChild? Aren't this making using elements very inflexible?

Imagine if we have a <my-modal> element, and its contents differ based on the type of users (for example, <user-table>, <paidUser-table>, <admin-table>, etc.):

With a non-dynamic approach, we would end up having to do something like <my-modal><user-table><paidUser-table><admin-table></admin-table></paidUser-table></user-table></my-modal> and then have to set display:block on <my-modal> and <paidUser-table>, as well as set display:none on all the other <x-table> to make this work if a paid user is using it, for example.

However, if elements are called dynamically, the structure can look something like this: <div id="modal><div id="table"></div></div> and then we can just use appendChild to insert the appropriate <x-table> into <div id="table>.

Am I missing a better approach for this? Is there any good reason that the pwa-starter-kit loads elements statically rather than dynamically?

Thanks!

itsjustbrian commented 5 years ago

PWA starter kit does load elements dynamically using dynamic import. This allows you to place the elements in the DOM, and then lazily instantiate their definitions. So in your example, you'd implement it exactly how you did in the "non-dynamic approach," but use dynamic import to load the correct table element definition depending on which user is looking at it. PWA starter kit uses this same pattern to load it's pages lazily: elements in DOM; calls to dynamic import.

freshgrapes commented 5 years ago

@fafcrumb thanks for the reply!

When I use the “non-dynamic approach”, (for example, loading <user-table></user-table> instead of <div id="table"></div>), the content of that element is immediately visible. That means everything is loaded without giving me the option to dynamically import the element with a function when I actually want it to be loaded.

In the scenario above, it means I will have to put all the element candidates in the html (<user-table>, <paidUser-table>, <...-table>, <...-table>, etc.) and ‘display: none’ the ones that shouldn’t be displayed for the current user type (for example, paidUser).

Imagine if I have 20 user types, meaning I will have to put 20 different element tags wherever I need a differentiated content, have them ALL be loaded, and hide 19 of them. Doesn’t this mean performance is going to suffer (since we have to load 19 other elements that won’t even be used), and also bad code maintainability (since instead of one dynamic element tag, we need 20, and that’s gonna happen whenever there are contents that differ based on user types)?

Dabolus commented 5 years ago

@freshgrapes you don't need to put all the element candidates into the HTML. Thanks to LitElement, you can easily choose what elements to add to the DOM and when. For example, in your use case instead of using appendChild you would do something like this:

render() {
  // This might a property in your element class or whatever
  const isUserAdmin = true;

  return html`
    <my-modal>
      <user-table>
        ${this.isUserAdmin ?
          html`<admin-table></admin-table>` :
          html`<paidUser-table></paidUser-table>`}
      </user-table>
    </my-modal>
  `;
}

Or, if you need to do something more complicated:

render() {
  return html`
    <my-modal>
      <user-table>
        ${this.getTable()}
      </user-table>
    </my-modal>
  `;
}

getTable() {
  switch (this.userType) {
    case 'admin':
      return html`<admin-table></admin-table>`;
    case 'paidUser':
      return html`<paidUser-table></paidUser-table>`;
    default:
      return html`<default-table></default-table>`;
  }
}

However, what's wrong in just not displaying the elements you don't want? Just displaying and undisplaying an element would actually be better then removing and re-adding elements to the DOM. For example:

render() {
  // This might a property in your element class or whatever
  const isUserAdmin = true;

  return html`
    <my-modal>
      <user-table>
        <admin-table ?hidden="${isUserAdmin}"></admin-table>
        <paidUser-table ?hidden="${!isUserAdmin}"></paidUser-table>
      </user-table>
    </my-modal>
  `;
}

Or even better, let the user-table element handle all this things using a property:

render() {
  // This might a property in your element class or whatever
  const userType = 'admin';

  return html`
    <my-modal>
      <user-table .type="${userType}"></user-table>
    </my-modal>
  `;
}

// And then handle the property inside the <user-table> element
freshgrapes commented 5 years ago

@Dabolus thanks so much! This is tremendously helpful and educational! I learned a lot about better programming practices just reading your examples!

However, what's wrong in just not displaying the elements you don't want? Just displaying and undisplaying an element would actually be better then removing and re-adding elements to the DOM.

Generally (not limited to this specific case but in a broad programming sense), when it comes to performance, is it better to display and hide the elements than adding and removing them from DOM even after the quantity of elements scales up?

Let’s say there are 10 user types, and so we will have 10 different elements (1 of them being displayed and 9 others being hidden). But what happens when the modal becomes complicated and there are numerous different modals? Like for example a modal with layers of layers of elements nested inside, then we are looking at possibly hundreds or even thousands of elements, and 90% of them are loaded yet unused and hidden.

Does it really not hurt the performance as much to load hundreds or thousands of unused elements than removing and only adding the comparably tiny amount of elements to and from the DOM every time it is needed?

Dabolus commented 5 years ago

@freshgrapes glad to help!


Generally (not limited to this specific case but in a broad programming sense), when it comes to performance, is it better to display and hide the elements than adding and removing them from DOM even after the quantity of elements scales up?

The DOM (Document Object Model) Tree is not the only tree the browser has to construct. The DOM Tree contains all of your HTML nodes, hidden or not. This means that all of your 10-20-whatever user types will be there. However, besides the DOM Tree, the browser also builds the CSSOM (CSS Object Model) Tree, that contains all the styles of your page. When both of this Object Model Trees are constructed, the browser also generates the Render Tree, which contains just the nodes that are actually visible on the screen (or better, the nodes that have to be rendered).

This means that if you use display: block/none (or the hidden attribute), you are adding/removing the element from the Render Tree, but not from the DOM tree. On the other hand, when you add/remove an element using appendChild/removeChild you are not only invalidating the Render Tree alone, but also the DOM Tree. This means that the browser has to construct again both the DOM Tree and the Render Tree. For this reason, I think that the hidden approach performs better, even though I don't have any benchmark that proves it.


Doesn’t this mean performance is going to suffer (since we have to load 19 other elements that won’t even be used)

Assuming that your elements start hidden (e.g. you set them hidden on first render), performance should not be going to suffer, as elements will be added to the DOM Tree, but not to the Render Tree (which is generally the one that causes performance issues).


the nodes that have to be rendered

Not directly related to your question, but definitely worth to know: the reason why I put this text in bold is that sometimes an element might be invisible to the final user, but still rendered by the browser. For example, when using visibility: hidden or opacity: 0 you are not removing the element from the Render Tree - you are just hiding it to the eyes of the user. This means that you should never use them if you have lots of elements (I don't see many use cases for them unless you have to create a semi-opaque element or you have to do some sort of appearing/disappearing animation).

freshgrapes commented 5 years ago

@Dabolus Thanks so much for such thoughtful reply!

Assuming that your elements start hidden (e.g. you set them hidden on first render), performance should not be going to suffer, as elements will be added to the DOM Tree, but not to the Render Tree (which is generally the one that causes performance issues).

What causes me all the worries is that I have used some apps/games that always perform great right after being opened, but then after 20 minutes of using them, they start to render things painfully slowly until I kill the apps/games and open them again, which again gives me great performance until another 20 minutes of using them (for example, a button would instantly brings up a modal when app is first opened, but after 20 minutes of interacting with the app, the same button would takes 3+ seconds to bring up that modal).

Facebook is NOT one of those apps discussed above, but think of its Profile Page. If a user is trying to find and friend all of his friends since kindergarten, he is likely going to open and close 1000+ profile pages, but he is not likely to return to any of them after opening them up.

In the above Facebook example, if we use the display: block/hidden/none approach, wouldn't there end up being 1000+ hidden and complex elements by the time this user is done searching for friends and start to do something else in the app (and he is not even going to return to those display: hidden/none profile pages anymore)? Wouldn't it be better if we just appendChild / removeChild every time the user opens a friend's profile and exits out of it?

itsjustbrian commented 5 years ago

@freshgrapes In that scenario, you wouldn't be using a different component for every single profile page. You'd use one component, and probably pass in the user you want to display through a property like so: <my-profile-page .user=${user}></my-profile-page>. Then you would show/hide that page with display: block/none. The profile page component renders the page by injecting the user's data into the same bit of DOM every time.

The scenario you're expressing a lot of worry over I think is valid when you have a potentially large and complex component/page that isn't necessarily ever going to be shown to the user. Something like an admin page for example. Assuming you've already downloaded this <admin-page> component into the browser, but the current user is not an admin, you're right that it would be potentially damaging to performance to add this to the DOM even though it never renders. Adding to the DOM tree isn't free of course. But this is a perfect scenario for conditionally rendering that page with LitElement like in the examples @Dabolus gave.

freshgrapes commented 5 years ago

@Dabolus @fafcrumb Thank you guys so much! You guys are making this community a great one! Hopefully I will be able to contribute back one day just like how you guys helped me!