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

Safari 11: Whole page re-render on navigate — what am I doing wrong here ⁉️ #636

Open
johnelliott opened this issue Mar 4, 2018 · 8 comments

Comments

@johnelliott
Copy link

johnelliott commented Mar 4, 2018

Expected behavior

Only 1 image request, modal renders but most elements untouched.

Actual behavior

every page image requested, whole body seems to re-render.

Steps to reproduce behavior

Navigate between two pages with an additional/less dom element tree.

Safari 11.03 exhibits the problem. Adding yo-yo-ify doesn't solve it.
Chrome only requests 1 image, and doesn't seem to re-render everything.


After months using this framework for my app I discovered that my nginx and caching were hiding the fact that the app was constantly re-rendering the entire body element and everything inside. It's requesting every image in my gallery grid each time I switched images in the Lightbox view.

I was under the impression that morphdom would compare the trees—which are nearly identical—and only render the Lightbox. I probably have something set up incorrectly, or I'm making incorrect assumptions about how the framework works. I have spent hours going over the handbook and figuring out server-side rendering and re-hydration, but now I'm running into another issue where I can't find any examples, guides, discussion, or any evidence this has been done before or anyone has opened the browser tools to see what is really going on.

I think the problem isn't that the framework is obtuse, but the the handbook and documentation are not adequate to onboard new users. There aren't examples of complex apps built with this.

screen shot 2018-03-03 at 7 49 22 pm

@johnelliott johnelliott changed the title Whole page re-render on navigate — what am I doing wrong here Whole page re-render on navigate — what am I doing wrong here ⁉️ Mar 4, 2018
@johnelliott
Copy link
Author

This is a stupid issue because I didn’t give sufficient contex to lead to a solution or fix, sorry.

@johnelliott johnelliott changed the title Whole page re-render on navigate — what am I doing wrong here ⁉️ Safari 11: Whole page re-render on navigate — what am I doing wrong here ⁉️ Mar 4, 2018
@yoshuawuyts
Copy link
Member

yoshuawuyts commented Mar 5, 2018

Heya, sorry this is happening! - the short answer is: we need to make use of the cache.

There's two options here, and it's up to you to pick the one that's best for you:

  • set cache headers: e.g. cache-control: immutable might make it so every request always hits your browser cache. It's necessary to use unique ids for each assets; for example by including the hash of an image as part of the resource name.
  • use nanocomponent: Choo's component abstraction. There's some nuances to it tho, and we haven't documented it in-depth yet - but it's the client-side response to handling this.

Hopefully this is somewhat helpful; apologies again you've run into this!

@johnelliott
Copy link
Author

johnelliott commented Mar 6, 2018

Hey @yoshuawuyts, thanks for responding. I was experimenting with the caching-dom-elements feature on Sunday. I was able to totally bungle up my UI and stop arbitrary things from refreshing, so that told me did in fact work. I think my issue is that I didn't have a tagged template string attached to a variable name that I could define isSameNode on. I reworked that so I had the function defined on just the image grid, but I still got the same results.

I think it makes sense to use isSameNode or a should-component-update-alike in certain cases where the user won't notice a difference, but I see evidence that the diffing works without my intervention with Chrome.

I tried to use yo-yoify and babel's env presets for a while, and in the process looked at the bundled files for a bit, but I was never able to narrow down exactly what was causing the different behavior in chrome or safari. As far as I can tell it's not a transpiling or bundling issue. I think that there may be something that doesn't work as intended with nanomnorph and safari (11.03 in my case). My hunch is that my particular Dom tree is pretty easy to diff since it's a series of uniquely-named images wrapped in similar tags.

Another possibility is that I have two image grids defined in two different page layouts, and that fact alone is causing a re-render regardless of whether they produce the same Dom elements. I am going to try to rule that out next, but someone more experienced than me on inspecting how the framework is working in Safari would be better at that task. :)

@yoshuawuyts
Copy link
Member

I updated choo-component-preview to v2 just now. Give it a shot if you're interested in caching components (: https://github.com/yoshuawuyts/choo-component-preview

@johnelliott
Copy link
Author

Caching manually sounds great, but why does it work properly in Chrome and not in Safari?

@tornqvist
Copy link
Member

tornqvist commented Mar 8, 2018

Here's a wild guess but are you by any chance using srcset? I noticed recently that due to how bel/yo-yoify transforms the template literals, attributes are applied in the order they are defined which causes Safari to prefetch the src before the srcset attribute is set.

So during initial page load the correct srcset-url will be fetched but as the view is being rerendered in the client, Safari will fetch the images src to, presumably, speed up page render.

var img = document.createElement('img')
// Safari will start fetching this right here ↓
img.setAttribute('src', '/some-image.jpg') 
// Chrome et.al., for some reason defers preload until here ↓
img.setAttribute('srcset', '/some-small-image.jpg 500w, /some-large-image.jpg 1000w') 

If this is the cause of your problem, simply changing the attribute order should solve the issue.

- <img src="/some-image.jpg" srcset="'/some-small-image.jpg 500w, /some-large-image.jpg 1000w">
+ <img srcset="'/some-small-image.jpg 500w, /some-large-image.jpg 1000w" src="/some-image.jpg">

Edit: also, if you have defined sizes, they should be defined first so that the correct srcset is used.

@johnelliott
Copy link
Author

johnelliott commented Mar 8, 2018

@tornqvist I didn't use srcset in this code, but I will give a closer look to the bundle when I get a chance. Your comment is making me think a closer evaluation of the bundle would be helpful.

Last night I ran a slightly older build on the RasPi. There I did not notice the bug with too many requests. This is exciting because now I can diff and exchange various pieces of the code to narrow down the source of the difference.

My next step is going to be to take the two copies of the app in each installation and diff the package-lock file and bundles, and perhaps trade bundles between machines to try to find what piece is the source of the isssue.

@johnelliott
Copy link
Author

After diffing the bundles I saw that babel was merely transforming the template strings and template functions, which safari supported anyway. With or without babel I got the same results.

I also learned that a cache header on the RasPi from nginx was eliminated the requests. The good news is that I can basically ignore this bug and use the cache headers to eliminate the requests.

The bad news is that I still don't know why Safari isn't working well with this version of Choo.

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

3 participants