salesforce / lwc

⚡️ LWC - A Blazing Fast, Enterprise-Grade Web Components Foundation
https://lwc.dev
Other
1.64k stars 392 forks source link

if-elseif-else fragment nodes are currently unoptimized #3084

Open jye-sf opened 2 years ago

jye-sf commented 2 years ago

Is your feature request related to a problem? Please describe. Currently, if-elseif-else fragment nodes are all generated as unstable. This means that any content inside these conditionals will have to go through the more expensive dynamic diffing algo. However, if a subtree doesn't contain any dynamic content, we should be able to diff them statically.

Describe the solution you'd like During the codegen for if-elseif-else nodes, we should determine whether a conditional subtree contains only static content. If it does, we should mark the fragment as stable, letting it go through the static diffing algorithm. This would be a nice performance optimization.

Describe alternatives you've considered

Additional context See the following for more context: https://github.com/salesforce/lwc/pull/3030#discussion_r983700309

git2gus[bot] commented 2 years ago

This issue has been linked to a new work item: W-11859388

nolanlawson commented 1 year ago

This optimization would be an observable change, yes @jye-sf? A component inside of an if/elseif/else would, instead of being unmounted+remounted, be patched instead?

jye-sf commented 1 year ago

instead of being unmounted+remounted

The main unmount+remount situation I can think of is dynamic components that change ctor/tag, but that only happens in the static diffing algo; I don't see an unmount+remount in the diffing algo. Was there another use case you had in mind?

Also, if/elseif/else currently shouldn't change how the diffing algo treats the children. This optimization shouldn't change that either, only which algorithm we end up using.

nolanlawson commented 1 year ago

I was thinking of this scenario:

<template lwc:if={foo}>
  <x-foo></x-foo>
</template>

AIUI, today <x-foo> will be unmounted/remounted when foo changes. With the proposed optimization, would it be merely patched?

jye-sf commented 1 year ago

I don't think anything should change in that case. The compiled component would look something like:

$cmp.foo
    ? api_fragment(0, [api_custom_element("x-foo", _xFoo, stc0)], 0)
    : null,

so when foo => false, the entire fragment would be unmounted, and when foo => true, remounted. I don't see that changing, since this optimization was primarily for the patchFragment use case.

jye-sf commented 1 year ago

Now that you bring it up though, I can see us doing an additional optimization for this use case so that, somehow, we cache and maintain the unmounted fragment. That has a few additional layers of complexity though, and then maybe stable should be used for only truly static content (text, element, comment, static nodes) disqualifying most if not all components.