nanohtml icon indicating copy to clipboard operation
nanohtml copied to clipboard

Network requests on each render

Open arturi opened this issue 8 years ago • 18 comments

Hi! Thanks for the module.

I have an example here: http://requirebin.com/?gist=f2a99cc27c465ed51eb8159830836ca4. In it, each time I call update and bel creates a new element, a local network request is made for the base64 inlined image, even though it did not change.

Should this be happening? I guess it makes sense, new element is created, bel is fetching it’s content. But that results in a poor performance — when I add a relatively large image, like 3mb, the Chrome browser tab can barely function and the interface becomes unusable.

To reproduce: navigate to http://uppy.io/examples/modal/index.html, click “open modal” and drag two files to the dragdrop area. Open network tab, click “upload”.

What am I doing wrong? Thank you!

arturi avatar May 26 '16 16:05 arturi

In itself it shouldn't be the worst the worst because the image is read out from cache each time, but I can imagine that you'd want to evade this scenario as much as possible regardless. You're indicating chrome freezes up for large images and that's definitely bad :(

An approach you can take is to memoize / wrap your function in a thunk. Fancy words for something that looks a bit like this:

var imgEl = null
function render (time) {
  if (!imgEl) imgEl = html`<img src="${img}">`
  return html`<div>
    <h1>${time}</h1>
    ${imgEl}
  </div>`
}

That way you no longer create a new request each time a render is passed. This should also work to optimize large sections of a tree, based on say changes in application state or other properties being passed.

I hope this was helpful. Cheers!

yoshuawuyts avatar May 26 '16 17:05 yoshuawuyts

@yoshuawuyts thank you! your solution works ✨ (proof: http://requirebin.com/?gist=0f7bb3b03959448e120f02d506f4cd7e)

However, made me think and try other solutions:

  1. React: http://esnextb.in/?gist=ec75c4fe3443dc16920a6d740415c090
  2. Virtual-DOM + hyperx + main-loop: http://requirebin.com/?gist=ecda5235da884da9335eb8029ccdcff9

Both work fine, no additional network requests. Which makes me wonder why the same won’t just work with bel. virtual-dom is smarter about creating new elements?

arturi avatar May 26 '16 21:05 arturi

This is an interesting problem! Both React and virtual-dom have access to the previous value. So they can check if the values are the same and skip the operation. bel doesn't have access to the previous value so it can't check if it's the same as last time to ignore the operation. It only creates elements and relies on morphdom or any other native DOM diffing library to handle the update.

For example we can't do the following when creating an element:

var previous = element.getAttribute(prop)
if (value !== previous) element.setAttribute(prop, value)

Because the element is always new and setting a src on a img element will initiate a network request.


AFAIK, appending to a document.createDocumentFragment() will still initiate the network request too.

I wonder if we should handle certain properties that can initiate requests or side effect events specially somehow. Or if we'll have to use an element registry. Or just leave it up to the framework implementations to handle this?

shama avatar May 26 '16 23:05 shama

The solution proposed by @yoshuawuyts is great, and seems like it would work well, but I think it'd be nice to make the problem hidden from developers. I'm not sure if that belongs in bel or a framework tho'.

Aside from src, are there other attributes with side effects that happen immediately upon being set?

jamesplease avatar May 26 '16 23:05 jamesplease

but I think it'd be nice to make the problem hidden from developers

Don't think that's possible - controlling re-renders is something that comes with the job I'm afraid. Be it through shouldComponentUpdate in React, manual diffing using thunks or setting lifecycle hooks to load and unload data. I reckon the best we can do is document it and make it as transparent as possible - that way we keep control which is a great thing.

I'll add docs for this in choo, as I reckon you're right and this is definitely a framework-level concern too. Thanks!

yoshuawuyts avatar May 27 '16 03:05 yoshuawuyts

Don't think that's possible - controlling re-renders is something that comes with the job I'm afraid

But, from a user/developer perspective, virtual-dom allows me to not think about this problem at all, meaning one less reason to use yo-yo and bel, even though I like them and appreciate the “real DOM instead of a virtual-dom” thing.

/cc @maxogden @substack, what do you think?

#tinylibrariesfatigue

arturi avatar May 27 '16 17:05 arturi

Each contributor has different motivations but for me, I couldn't care less about competing with React or virtual-dom. I use yo-yo because it solves a very specific problem for me: I hate peer dependencies.

At work we use Ember, it's a massive app with over 400 components. It has literally taken a year and wading through a ton of merge conflicts to finally upgrade to 1.13. The latest Ember is now 2.5 so who knows if our app will ever actually catch up.

This isn't Ember's fault, it's an inherit problem with peer dependencies when you maintain a large number of components. React and virtual-dom have this issue too. One could argue the free upgrades we get from the framework makes it worth it. It's hard to actually know if all that time spent keeping up would have been better specifically improving our components instead.

Outside of work, I don't have time to keep up with a framework. So I use yo-yo for everything. I can leave a project for a year and assuming the browser didn't break my code, it will still work with new modules I create.

I know this doesn't solve this particular issue but thought I'd share my own motivation behind the native DOM approach.

shama avatar May 27 '16 19:05 shama

I’m totally on board with your motivation. We went with yo-yo in Uppy because we are building a file uploading library and wanted something simple and lightweight. And we’ve been generally happy with it, so thanks again. I plan to use it in my personal projects as well, once I get a hold of things.

arturi avatar May 31 '16 21:05 arturi

I think it would be great to hide this from developers. Obviously what @yoshuawuyts said about this coming with the territory is part of working with the real DOM, but also I think thats a cop-out answer when we haven't explored all the possibilities.

Two possible workarounds:

  • wrap everything in a <noscript> tag before passing to morphdom, which disables all scripts as well as image loading, then have morphdom only operate on the inner tree (bypassing the
  • rewrite the img url to be something like morphdom-http://originalurl.com and then have morphdom rewrite it when it gets inserted into the dom

max-mapper avatar Jun 25 '16 17:06 max-mapper

@maxogden I'd be careful with tying bel to morphdom specifically; the more native DOM abstractions we use, the more generic and thus reusable the solution is. While i see a place for the solutions you're proposing, they wouldn't, for example, work with React's render loop or any other vdom implementation.

Right now morphdom seems to bench on par with react - with virtual-dom sitting at 2x the speed of that; and the fastest vdom implementation benching way above that. While morphdom works great right now; I think we can all agree that it'd be great if we can finally get around to writing browser elements that outlast more than a single cycle of frameworks.

edit: that's not to say that I don't agree with you that it would be great if we could abstract this away from devs; we should def explore these options! What I'm trying to say is we should be cautious in using abstractions that diverge from the DOM, as relying on them might come back to bite us

yoshuawuyts avatar Jun 25 '16 19:06 yoshuawuyts

@yoshuawuyts why wouldn't wrapping it in a <noscript> work everywhere?

max-mapper avatar Jun 25 '16 20:06 max-mapper

BTW made an experiment to test the noscript theory and I can't get it to disable requests in chrome or FF so maybe that isn't even worth discussing further http://requirebin.com/?gist=8dc9eebaefdb1ca0d04684c7095619d6

max-mapper avatar Jun 25 '16 21:06 max-mapper

why wouldn't wrapping it in a

Oops; I interpreted it as though morphdom would inject behavior surrounding this - which wouldn't be transparent. If morphdom isn't the one adding this beahvior I'm def all :+1:

yoshuawuyts avatar Jun 25 '16 22:06 yoshuawuyts

Found another problem with the caching img element solution to prevent extra requests. For some reason when using pre-created element saved in a variable, entire element tree where that pre-created element is then used gets re-rendered in DOM.

Please see here: http://requirebin.com/?gist=0f7bb3b03959448e120f02d506f4cd7e. If you use inspector to compare behaviour of the two img elements — the first one gets thrown away by morphdom, preventing animations (the first animation can’t finish) and click events (which might fire in between those DOM changes). The second stays in place like it should.

arturi avatar Aug 06 '16 03:08 arturi

Think this might be related to https://github.com/shama/on-load/issues/10, which seems like an issue in morphdom if I understood the thread well

yoshuawuyts avatar Aug 06 '16 09:08 yoshuawuyts

Reported to morphdom at patrick-steele-idem/morphdom#77

timwis avatar Aug 06 '16 15:08 timwis

What do you think of this solution by to the “src network requests” problem: https://github.com/patrick-steele-idem/morphdom/issues/77#issuecomment-238719432?

arturi avatar Aug 12 '16 19:08 arturi

Waiting on patrick-steele-idem/morphdom#77 to be published; made a demo on how to create widgets with the isSameNode() API here: shama/on-load#10 (comment) (e.g. thunking!)

That patch should fix this issue :tada:

yoshuawuyts avatar Sep 10 '16 15:09 yoshuawuyts