sveltejs / sapper

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

<script>'s in <svelte:head> are running twice (unless wrapped in {@html}) #1124

Open notdotscott opened 4 years ago

notdotscott commented 4 years ago

Describe the bug Adding a <script> tag to <svelte:head> causes the script to be run twice. I believe this is down to Sapper generating the SSR code and then later on hydrating the page with the same element. However, wrapping the script in a @html directive only runs the script once (it looks like the hydration isn't duplicating the element).

Logs N/A

To Reproduce

I have created a small repo (using sapper-template) that reproduces the issue: https://github.com/notscottthompson/sapper-head-script-test

In the <Nav> component I have added the following:

<svelte:head>
  <script>console.log('hello from Nav');</script>
  <script src='some-script.js'></script>
  {@html '<script src="some-other-script.js"></script>'}
</svelte:head>

some-script.js logs "hello from some-script.js". some-other-script.js logs "hello from some-other-script.js".

The following is logged in the browser when loading the test page:

hello from Nav
hello from some-script.js
hello from some-other-script.js
hello from Nav
hello from some-script.js

Expected behavior I would expect the following to be logged:

hello from Nav
hello from some-script.js
hello from some-other-script.js

Stacktraces N/A

Information about your Sapper Installation:

Severity Only mildly annoying. However, due to not being an expert in Svelte/Sapper I'm worried that this may be something I shouldn't be doing (wrapping scripts in @html) or that there may be a better way to achieve it.

Additional context N/A

jervtub commented 4 years ago

Hi, I have tried to replicate this problem, but I didn't manage to do so. However, I didn't copy code one-on-one from the small repo you have linked. In the pull request I have add a hydration test.

jervtub commented 4 years ago

Updating src/server.js, src/client.js, src/template.html, src/routes/index.svelte doesn't result in an error either.

notdotscott commented 4 years ago

Hi @jervtub, I've just tried with a fresh clone of my sample repo and updated both Svelte and Sapper to the latest versions ("sapper": "^0.27.13", "svelte": "^3.22.3") and the issue remains. Here's a screenshot of what I see in the dev tools console:

Screenshot 2020-05-18 at 09 49 37

jervtub commented 4 years ago

I have checked out your repo, the problem indeed occurs, my problem. I'll take another look!

Smilefounder commented 4 years ago

Got the same issue, and jquery load twice. @jervtub could you help let us know if the issue has been resolved?

jervtub commented 4 years ago

Unfortunately, I haven't had time to continue with bug resolving due writing my thesis. To resolve this, this might be more complex than I initially expected (I am still new to the code base of Sapper and Svelte). Maybe the core development team can take a closer look. Furthermore, today I have found a decent amount of the time to attempt tackling it once again.

benmccann commented 4 years ago

I expect this would be fixed by https://github.com/sveltejs/svelte/pull/4309. You could test to see if using https://github.com/wix-incubator/wix-svelte fixes this for you, which is a fork by that author containing the patch

Smilefounder commented 4 years ago

Hi @benmccann confirmed the issue has been resolved with your version. Thanks alot.

I expect this would be fixed by sveltejs/svelte#4309. You could test to see if using https://github.com/wix-incubator/wix-svelte fixes this for you, which is a fork by that author containing the patch

Smilefounder commented 4 years ago

Hi @benmccann sorry, After tried several times, The issue still happening.

image

rcauquil commented 4 years ago

I have the same issue, firebase + stripe are loaded twice when added in svelte:head (even in only one place)

benmccann commented 4 years ago

Can you see if this still happens if you upgrade to 0.28.0 and set hydratable: true: https://github.com/sveltejs/sapper-template/blob/b51e2f2d9769b4a9d2b9e63c9177ce587bee7a9a/rollup.config.js#L75

Head hydration was improved and I wonder if that would make a difference here. If that doesn't work, it'd be interesting to test if the svelte hydration fix mentioned above https://github.com/sveltejs/sapper/issues/1124#issuecomment-647121837 now works with these improvements applied

orhanmaden commented 4 years ago

That fixed the issue for me. Can this setting have side effects?

Can you see if this still happens if you upgrade to 0.28.0 and set hydratable: true: https://github.com/sveltejs/sapper-template/blob/b51e2f2d9769b4a9d2b9e63c9177ce587bee7a9a/rollup.config.js#L75

Head hydration was improved and I wonder if that would make a difference here. If that doesn't work, it'd be interesting to test if the svelte hydration fix mentioned above #1124 (comment) now works with these improvements applied

tommywalkie commented 4 years ago

For reference @benmccann I've linked a issue reproduction repo in https://github.com/sveltejs/svelte/issues/4982 which seems to be the upstream issue

Made a repo (svelte-head-ssr-dupe-repro) using the default Sapper template for reproducing the issue.

In src/routes/index.svelte I just added the following code :

<script>
    let filename
    const getCurrentFile = input => {
        const split = input.split(/[\\/]/)
        return split.splice(split.length - 2, split.length).join('/')
    }
    try {
        // Should output `server/server.js` if run inside Node
        filename = getCurrentFile(__filename)
    } catch (error) {
        // Should output something like `client/client.c11ed98a.js` if run inside browser
        filename = getCurrentFile(document.scripts[document.scripts.length-1].src)
    }
    const style = `<style location="${filename}">p { color: blue; }</style>`
</script>

<svelte:head>
    <title>Sapper project template</title>
    {@html style}
</svelte:head>

When running the Sapper dev server, this is the following output in <head> section :

<style location="server/server.js">p { color: blue; }</style>
<style location="client/client.4cce6104.js">p { color: blue; }</style>
thebrianbug commented 3 years ago

Was pulling my hair out over this. It would be great to fix or document it if we don't want to pull resources from SvelteKit :)