Skip to content

[discussion] investigate vanillajs approaches #754

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

Closed
wants to merge 27 commits into from

Conversation

nweldev
Copy link
Contributor

@nweldev nweldev commented Jun 19, 2020

This PR is for feedbacks and discussions only

follows #749 (comment) & #749 (comment)

As I'm currently using this project in order to investigate different VanillaJS practices and their impacts on rendering performance, I'm creating some new poorly optimized implementations. Those aren't intended to be merged as is, as they are out of this project's scope.

Yet, sharing those implementations with contributors to this project may help some discussions about the current ones.

Especially:

  • I would be happy to make (based on vanillajs-wc-dx, but in another PR) the vanillajs-wc implementation follow good practices in order to be closer to "real-life" projects, even if it means worse results.
  • I'm currently trying to list as many vanillajs rendering approaches and their impact on performance. So any advice, feedback, and help are more than welcome.

FYI: in order to allow me to follow how rendering approaches compares without creating a bunch of implementations, I commit every results, so of course, those are conflicting with the master branch.

@nweldev nweldev force-pushed the compare-vanillajs branch from 3f3ac30 to be30c49 Compare July 4, 2020 16:18
@ryansolid
Copy link
Contributor

@noelmace Awesome work on so many variants. Any observations so far?

@nweldev
Copy link
Contributor Author

nweldev commented Jul 4, 2020

Whoops, I forgot about this PR 😅 Sorry for the flooding, here is a quick summary of some of my observations so far.

the "good" VanillaJS approach

So, first, the generally advocated VanillaJS approach is kind of dumb, as it leads to ~30% worse rendering performance than an optimized one, and even ~10% compared to most rendering micro-libs (e.g. lit-html).

Plus, it's less than 10% better than Preact & Co, which offers a way better DX.

This is mostly because:

  • using innerHTML w/ a template sting is ~10% (9-13%) slower than the programmatic approach (createElement etc.)
  • just using a custom element makes rendering ~10% (7-15%) slower
    • w/o even following good practices like reflecting properties and attributes (which makes it even slower)

I'm still investigating different "minimal vanillajs component" approaches (no custom element, pre-parsed or programmatic template, no advanced function, etc...) w/ some helper functions.

But I'll need to work on a solution to test shadow dom to compare those with "real life" Web Components.

Some other general considerations

  • it's generally better to use just one global event handler instead of local ones
    • even if the callback becomes very slow, performance is better most of the time
    • DX is often better
    • this is mostly related to templating/rendering, as setting local event handlers often leads to more code
    • this is just a global constatation, I'll need to give more thoughts to that
  • use a "DOM buffer" (i.e. store the elements in collections) when you can, it can increase performance a lot

In general

  • textContent = "" is always the best solution
    • but parentNode.replaceChild() leads to pretty much the same performance
  • requestAnimationFrame can lead to better rendering performances
    • but not enough to make it very useful considering to awful associated DX (except for specific use cases of course)
  • append() is slightly faster than 2 or more appendChild() (yeah, obviously 🤷)

Some comments on the pre-existing examples

  • "selection" with lit-html is very slow, yet this doesn't seem to be related to lit-html itself. I'll investigate the actual demo later, let me know if you have any idea...
  • globally, vanillajs only uses "one-time rendering components" for rows, which IMO is kind of misleading. For example, if I add any disconnectedCallback() to a row custom element, this slows down clear by x2. I can remove it, but only because I don't have to care about the component being reconnected after that in this specific use case. IMO, this is far from a "real use case" example
  • vanillajs-wc always skipp rowId = item.id and selectedRow = null, which makes it a little bit inconsistent with some other examples

@nweldev nweldev changed the title (WIP) investigate vanillajs approaches (WIP) [discussion] investigate vanillajs approaches Jul 4, 2020
nlm-pro added 22 commits July 5, 2020 23:42
requires:
- to use an autonomous custom element named benchmark-row with
a shadowRoot containing one HTMLTableRowElement
- to add set `useRowShadowRoot` to `true` in "js-framework-benchmark"
in the integration's package.json

results for partial update, swap rows & remove row seems seems wrong
as they are way too low
DX first: uses innerHTML with bad event listeners.

The row component is badly optimised because it can't use a shadow dom.

Delete in incredibly slow because it needs to do a findByIndex for
everything (but maybe it's cheating to do otherwise, as vanilla-wc do
the same).

Optimisations:
- the main component stores the rows elements in order to avoid to
  access the DOM too often
- the main component uses a template to minimise parsing
remove disconnect has to makes clear run slower
only listen on click at the root of the row component
use append instead of appendChild to optimise DOM access
new implementation based on vanillajs-wc-dx but using createElement
instead of innerHTML
@nweldev nweldev force-pushed the compare-vanillajs branch from 41710b6 to ba7e11b Compare July 5, 2020 21:52
@nweldev
Copy link
Contributor Author

nweldev commented Jul 7, 2020

I made a lot of changes in my fork, and this branch has been removed in favor of fullwebdev/new-vanillajs-implementation.

Please check the dedicated issue I created to discuss differences between this fork and this project: https://github.com/fullwebdev/benchmark/issues/1

@nweldev nweldev closed this Jul 7, 2020
@nweldev nweldev deleted the compare-vanillajs branch July 7, 2020 18:15
@nweldev nweldev changed the title (WIP) [discussion] investigate vanillajs approaches [discussion] investigate vanillajs approaches Jul 7, 2020
@nweldev nweldev restored the compare-vanillajs branch July 21, 2020 21:24
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

Successfully merging this pull request may close these issues.

3 participants