sveltejs / sapper

The next small thing in web development, powered by Svelte
https://sapper.svelte.dev
MIT License
7k stars 434 forks source link

Duplicated Elements in Production Build #1725

Open MTyson opened 3 years ago

MTyson commented 3 years ago

I have markup like so:

            <nav>
    <ul>
        <li><a href="/">
            <span style="color: orangered; background: -webkit-linear-gradient(orangered, darkorange); -webkit-background-clip: text;-webkit-text-fill-color: transparent;; font-weight:bold;">The</span> 
            <Icon>
                <svg version="1.1" id="Layer_1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px" 
                    viewBox="0 0 122.88 102.72" 
                    style="enable-background:new 0 0 122.88 102.72; color: orangered; background: -webkit-linear-gradient(orangered, darkorange); 
                    -webkit-background-clip: text;-webkit-text-fill-color: transparent;; font-weight:bold;" xml:space="preserve">
                    <g><path class="st0" d="M65.61,20.91v72.74h35.63c0.38,0,0.68,0.31,0.68,0.69v7.7c0,0.38-0.31,0.69-0.68,0.69H22.84 c-0.38,0-0.69-0.31-0.69-0.69v-7.7c0-0.38,0.31-0.69,0.69-0.69h35.63l0-72.71c-3.1-1.08-5.56-3.53-6.64-6.63H29.3v3.43 c0,0.38-0.31,0.68-0.68,0.68h-5.78c-0.38,0-0.69-0.31-0.69-0.68v-3.43h-5.73c-0.44,0-0.8-0.31-0.8-0.68V7.84 c0-0.38,0.36-0.69,0.8-0.69h35.43C53.33,2.99,57.31,0,61.99,0c4.68,0,8.66,2.99,10.14,7.16h35.53c0.44,0,0.8,0.31,0.8,0.69v5.78 c0,0.38-0.36,0.68-0.8,0.68h-6.46v3.43c0,0.38-0.31,0.68-0.68,0.68h-5.78c-0.38,0-0.69-0.31-0.69-0.68v-3.43H72.16 C71.09,17.38,68.67,19.81,65.61,20.91L65.61,20.91z M99.66,22.3l22.91,40.48c0.2,0.35,0.29,0.73,0.28,1.1h0.02c0,0.05,0,0.1,0,0.15 c0,9.64-11.35,17.46-25.35,17.46c-13.85,0-25.1-7.65-25.34-17.15c-0.04-0.16-0.06-0.34-0.06-0.51c0-0.44,0.14-0.86,0.37-1.2 l23.43-40.43c0.59-1.02,1.89-1.37,2.91-0.78C99.2,21.65,99.48,21.95,99.66,22.3L99.66,22.3z M99.75,31.11v30.6h17.32L99.75,31.11 L99.75,31.11z M95.67,61.7V31.16L77.96,61.7H95.67L95.67,61.7z M27.54,22.3l22.91,40.48c0.2,0.35,0.29,0.73,0.28,1.1h0.02 c0,0.05,0,0.1,0,0.15c0,9.64-11.35,17.46-25.35,17.46c-13.85,0-25.1-7.65-25.34-17.15C0.02,64.19,0,64.02,0,63.84 c0-0.44,0.14-0.86,0.37-1.2L23.8,22.21c0.59-1.02,1.89-1.37,2.91-0.78C27.08,21.65,27.36,21.95,27.54,22.3L27.54,22.3z M27.63,31.11v30.6h17.32L27.63,31.11L27.63,31.11z M23.54,61.7V31.16L5.84,61.7H23.54L23.54,61.7z M61.99,6.07 c2.59,0,4.69,2.1,4.69,4.69c0,2.59-2.1,4.69-4.69,4.69c-2.59,0-4.69-2.1-4.69-4.69C57.3,8.17,59.4,6.07,61.99,6.07L61.99,6.07z"/></g></svg>
            </Icon>
            <span style="color: orangered; background: -webkit-linear-gradient(orangered, darkorange); -webkit-background-clip: text;-webkit-text-fill-color: transparent;; font-weight:bold;">Referendum</span>
            </a>
        </li>

In my nav.svelte component. This works fine in dev.

However, in production, there is often a strange duplication of many elements. This is not consistent, but common. It is worse on remote cloud deployment of the app, but also seen on local running of prod build.

Other elements are strangely reproduced, but this is an easy instance you can see. Notice the word "referendum: is displayed twice, but only in the markup once.

sapper-prod-dupe

The app is live here, you might need to reload a couple times to see it. http://35.223.159.8/

MTyson commented 3 years ago

I have been troubleshooting, and it has something to do with the SVG elements. If they are removed from the app, the problem goes away.

MTyson commented 3 years ago

Here is minimal repro: https://github.com/MTyson/sapper-bug Just the starter template with a couple icons in it. It is running here: http://rfrndm.com/ Just reload a couple times and you'll see it.

MTyson commented 3 years ago

image

MTyson commented 3 years ago

Where would one start debugging this? I can start digging in the code.

I guess when sveltekit comes out this will be obviated...

bardobrado commented 3 years ago

It only happens on chrome base browser..

I`m not having this issue on firefox, but in chrome, all mess up.

antony commented 3 years ago

I'm not sure what you're doing here, but I can't reproduce your issue, using chrome, or otherwise.

What I did notice on refresh is some severe layout shift, as if you're not fully understanding how SSR works, and are doing a lot of your work in the client side. It appears that all of your content renders, and then some css or js hides it all so that only the view you're looking at is shown.

It's possible that something in your javascript is breaking the client-side and that's why you're getting a double render.

If you can follow the issue template and fill in the required sections as requested it will be easier to help you.

bardobrado commented 3 years ago

I stayed the hole night trying to figure out if it was something wrong with my code, but happens that if I remove the images from Nav component, and place any other thing, the problem is finished. As soon as i put 2 different images, every thing rips apart. It gets easy be replicated with sapper template, and add 2 or 3 different images to Nav.svelte.

This bug happens on chrome and firefox just check below.

You can test here for a while: https://mayarahto.com

After some chat, i think that sapper interpreter are trying to find < / img ">, but images doesnt finish like that, or we get an error, imgs finish />... so sapper bugs trying to find this tag that doesnt ecxists in html.

softgripper commented 3 years ago

^ I do not know that it's the image tag. I have seen the bug, and it's weird and inconsistent.

antony commented 3 years ago

There's so much going on with that site that I don't know where to begin.

I loaded it the first time in a fresh browser and I got a navigation bar with pink icons. I loaded it again and I got duplicated icons (such as you describe in the issue) Then I deleted the service worker and got a NGINX error

Screenshot from 2021-03-09 09-34-24

Then when I loaded it again a couple of times I got a different site

Screenshot from 2021-03-09 09-34-42

needless to say the error isn't recreatable any more, since the site with the issue seems to have disappeared.

I'm afraid there seems to be a need for more work to isolate and reliably reproduce this issue - it's a bit of a moving target at the moment.

You might remove the service worker to reduce the complexity of the app.

bardobrado commented 3 years ago

Sorry, i was testing other versions of sapper to see if it was without this problem, but my vps crashed and i have to rebuild it, in the meanwhile i got out of light in my building. I'll re-build the server again and warn you about it. But it is simple, where is writen on sapper template, Home, About, Blog, between "a" tag, change it, placing 3 different images, and you get the bug. Anyway..

antony commented 3 years ago

The way to create a repro for this is:

Get the buggy repro locally, make it as small as humanly possible so that the bug is really isolated, and run npm run build and then npx http-server __sapper__/build. If you can see the issue there, then commit the repro to github, so that we can do the same and have a look.

Beyond that, there are way too many things going on here to try to fix the issue.

bardobrado commented 3 years ago

Done it! http://mayarahto.com/ - for test And the repo, to take a look: I just changed Nav.svelte https://github.com/bardobrado/sample_sapper

babeard commented 3 years ago

I am also seeing this in production. As for how to reproduce with @bardobrado's repo, the following may help:

Also, by disable cache in dev-tools, it completely removes the issue. Caching / rehydration issue?

MTyson commented 3 years ago

What I did notice on refresh is some severe layout shift, as if you're not fully understanding how SSR works, and are doing a lot of your work in the client side. It appears that all of your content renders, and then some css or js hides it all so that only the view you're looking at is shown.

It's possible that something in your javascript is breaking the client-side and that's why you're getting a double render.

If you can follow the issue template and fill in the required sections as requested it will be easier to help you.

To create that later example, all I did was take the standard template and put a couple SVG in the navbar.

We basically put production on hold and are awaiting sveltekit hoping that will fix this.

Thanks for taking a look @antony

MTyson commented 3 years ago

I stayed the hole night trying to figure out if it was something wrong with my code, but happens that if I remove the images from Nav component, and place any other thing, the problem is finished. As soon as i put 2 different images, every thing rips apart. It gets easy be replicated with sapper template, and add 2 or 3 different images to Nav.svelte.

I'm just glad we aren't the only team experiencing this.

A real nightmare.

I was able to repro the bug on your site, @bardobrado

image

MTyson commented 3 years ago

Sorry, i was testing other versions of sapper to see if it was without this problem, but my vps crashed and i have to rebuild it, in the meanwhile i got out of light in my building. I'll re-build the server again and warn you about it. But it is simple, where is writen on sapper template, Home, About, Blog, between "a" tag, change it, placing 3 different images, and you get the bug. Anyway..

We tried rolling back to earlier versions of Sapper/Svelte (and related plugins), but it still repro'd the bug.

antony commented 3 years ago

Which version(s) of Sapper did you go back to?

MTyson commented 3 years ago

Which version(s) of Sapper did you go back to?

I don't recall exactly all the versions, but we are on sapper 0.29.1 right now, and we went back to 0.28.x for sure. I'm pretty sure we tried 0.27.x also. (We also changed svelte and the other related deps).

bookyo commented 3 years ago

59198F20-2989-47C6-934E-AFAF5CC7AFC3 CD2E52E7-EAF3-4EAD-A4D7-EA9996467598 same issue! this my demo:http://198.16.50.194:3000 just ctrl+r refresh some times in brower, and you will see the bug! i dont know what happen!

bookyo commented 3 years ago

one month ago , that not happen.

babeard commented 3 years ago

FYI: Not that this issue shouldn't be fixed on the sapper side of things, but I couldn't reproduce this issue with svelte-kit.

MTyson commented 3 years ago

FYI: Not that this issue shouldn't be fixed on the sapper side of things, but I couldn't reproduce this issue with svelte-kit.

Thank god.

bookyo commented 3 years ago

I found some interesting things. The service-worker did not work normally on the page where the problem occurred, but after reverse proxy a domain name, I refreshed it again and there was no problem, and the service-worker started to work normally! When I checked the network, I found this problem. If necessary, I can provide my server for official review. this is ip:port : 607D3C78-16BA-4E78-86A8-D61C6E35CD72 this is reverse proxy a domain: F127A6C8-BAE0-49CB-ABC5-6D1D19A65DCD

bookyo commented 3 years ago

Same server: https://cms.beefun.cc

http://202.182.110.239:3001

you can try 3001 port, refresh page and will see the bug! 455CEEE2-FA16-449D-A7D9-2619E8F3D649

but if you visit cms.beefun.cc, refresh page and work fine!

@MTyson i can provide my test server if you want and my code is open source, so dont worry.

webfrank commented 3 years ago

Hi, just got same problem upgrading from 0.27.x to 0.29.x.

I have a login page with just a button and if I reload the page I got two buttons, side by side.

In the svelte page I have a simple Login tag.

antony commented 3 years ago

Right, so it makes sense why I'm not seeing this now. I took a fork at Sapper 0.27.* and have been running off this. It looks like this was introduced somewhere after 0.28.

My advice is, if you've upgraded to Sapper 0.28 or later, I would revert that upgrade. We would also welcome help from the community in diagnosing and fixing this issue, since our resources to do so are very limited right now. If a fix is made for this issue we will create a new release to publish it, despite Sapper being in maintenance mode.

MTyson commented 3 years ago

@antony - where would you start in debugging this?

babeard commented 3 years ago

I think I have found the solution.

The reason why 0.28.0+ was because https://github.com/sveltejs/sapper-template/pull/247/commits/d4c75af7f2392af1a635210ef6da4581f7713a7f#diff-4b1d306eb7949b768a10a68600e5973cdcbeafa0827e6f6e3c78c55269d4cc0b .

I believe this causes the sapper script to be evaluated before the DOM has actually been properly mounted on slower connections, thereby creating duplicate elements. This becomes more pronounced the larger the client script is (i.e. base64ed images). It's the best explanation I've got anyways.

MTyson commented 3 years ago

I think I have found the solution.

  • Move %sapper.scripts% within src/template.html from the head to the end of the body.

The reason why 0.28.0+ was because sveltejs/sapper-template@d4c75af#diff-4b1d306eb7949b768a10a68600e5973cdcbeafa0827e6f6e3c78c55269d4cc0b .

I believe this causes the sapper script to be evaluated before the DOM has actually been properly mounted on slower connections, thereby creating duplicate elements. This becomes more pronounced the larger the client script is (i.e. base64ed images). It's the best explanation I've got anyways.

Bad ass!!

babeard commented 3 years ago

It is also worth repeating in this thread that a more elegant solution, suggested by @benmccann, is wrapping your sapper.start() in a window load event listener.

// src/client.js

window.addEventListener('load', () => sapper.start({
  target: document.querySelector('#sapper')
}));
webfrank commented 3 years ago

Hi, I can confirm @babeard solution is working, no more duplicated content.

MTyson commented 3 years ago

Hi, I can confirm @babeard solution is working, no more duplicated content.

Likewise, using the load event in client.js fixed the problem.

benmccann commented 3 years ago

Could people test if https://github.com/sveltejs/sapper/pull/1722 fixes this?

babeard commented 3 years ago

Unfortunately, no. I tested that a week ago and again just now and no dice.

According to MDN

The defer attribute has no effect on module scripts — they defer by default.

MTyson commented 3 years ago

I just saw a variation of this bug after not seeing it for a while (after using this fix: https://github.com/sveltejs/sapper/issues/1725#issuecomment-801559348)

Seems to be mostly fixed though.

babeard commented 3 years ago

Hmm. @MTyson did you use the window load event method or the append %sapper.scripts% to body method? Curious to see if one works better than the other.

MTyson commented 3 years ago

Hmm. @MTyson did you use the window load event method or the append %sapper.scripts% to body method? Curious to see if one works better than the other.

I used the window.addEventListener('load' in client variation... I'm going to try the other next.

zwergius commented 3 years ago

Hi,

Just want to add to this that adding window.addEventListener('load' solved an issue I was having when refreshing Safari with a normal cmd+r where almost never the client code would kick in (I had an onMount with a console log in a layout file). This in my case was only an issue in Safari, both desktop and iOS

asdfdelta commented 3 years ago

We recently had the duplication with components with SVGs. Strangely, it was caused by the hydration because the original page source didn't contain any duplication.

We found that there was a <link> tag inside of a component outside of <svelte:head> tags. I think that screwed up the selectors of elements and upon hydration it duplicated the elements. Putting the link tag into the head fixed our issues immediately.

MTyson commented 3 years ago

Thanks @asdfdelta , that is a good find.

ToP29 commented 3 years ago

In my case, when I used saper with ssr, it ignored first element inside <div id="sapper"> when hydratating content. So i just added sacrificial <div style="display:none;" /> and everything works now.