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

__layout.svelte may run twice after browser reload in dev #2130

Closed
jsprog opened this issue Aug 8, 2021 · 22 comments · Fixed by #4891
Closed

__layout.svelte may run twice after browser reload in dev #2130

jsprog opened this issue Aug 8, 2021 · 22 comments · Fixed by #4891
Labels
bug Something isn't working p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Milestone

Comments

@jsprog
Copy link

jsprog commented Aug 8, 2021

Describe the bug

This issue only affects the context script for src/routes/__layout.svelte. Other layout files are not affected, and #1214 is different.

At first, everything works fine across browser reloads, updating some other files isn't a problem at all. But updating the top level layout file, or any of it's imported dependencies (components or modules) will trigger this issue, to cause the context script to run twice when I reload the browser's tab. Clearing the browser's cache won't help until I reload the development server.

Before the update
before update

After the update (notice the abnormal additional log changing positions)
update   reload 1
update   reload 2
update   reload 3

Reproduction

  • sveltekit skeleton project: npm init svelte@next __layout-ctx-run-twice
  • add the top level layout file containing:
<!-- src/routes/__layout.svelte -->
<script context="module">
    console.log('%c module: this may runs twice.', 'color: purple; font-weight: bold;')

    export function load () {
        console.log('%c load: this runs once.', 'color: magenta; font-weight: bold;')
        return {}
    }
</script>

<script>
    console.log('%c instance: this runs once.', 'color: blue; font-weight: bold;')
</script>

<slot></slot>
  • start the development server: npm run dev
  • navigate to localhost:3000, all the logs are logged once respecting the same order across reloads, and they're all coming from __layout.svelte? [sm]
  • force update src/routes/__layout.svelte, then reload the browser tab.
  • inspect the logs again to notice the first log in code (purple) is logged twice. One is coming from __layout.svelte? [sm] as the other logs, and the abnormal one is coming from __layout.svelte
  • reload as much as you like and you'll notice that the abnormal log may not respect the same log order.
  • finally, reload the development server, and the issue will disappear.

Logs

No response

System Info

System:
    OS: Linux 4.19 LMDE 4 (debbie) 4 (debbie)
    CPU: (4) x64 Intel(R) Core(TM) i5-3570K CPU @ 3.40GHz
    Memory: 3.28 GB / 15.56 GB
    Container: Yes
    Shell: 5.0.3 - /bin/bash
  Binaries:
    Node: 14.17.1
    npm: 6.14.13
  Browsers:
    Chrome: 87.0.4280.141
    Firefox: 68.8.0esr

Severity

annoyance

Additional Information

Tested using both Chrome and Firefox.

@benmccann benmccann added this to the 1.0 milestone Aug 9, 2021
@benmccann benmccann added the p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. label Aug 9, 2021
@vwkd
Copy link

vwkd commented Aug 10, 2021

I can reproduce using the following __layout.svelte.

<script context="module">
    console.log("Layout module.");
</script>

<script>
    console.log("Layout instance.");
</script>

Start dev server as normal. Change something in __layout.svelte that triggers hot reloading. Then refresh page in browser.

This issue seems to affect hot reloading of the dev server, as it disappears after restarting the dev server.

@rmunn
Copy link
Contributor

rmunn commented Aug 10, 2021

#2154 (comment) may have identified the cause of the issue. All the components (layout, error, and pages) in manifest.js are written in "delayed" function so that they won't be imported until actually needed:

const c = [
	() => import("..\\..\\..\\src\\routes\\__layout.svelte"),
	() => import("..\\components\\error.svelte"),
	() => import("..\\..\\..\\src\\routes\\index.svelte"),
// ...
];

export const routes = [
	// src/routes/index.svelte
	[/^\/$/, [c[0], c[2]], [c[1]]],
// ...
];

But the fallback array at the bottom of manifest.js is different: it immediately calls the functions for the layout and error page, importing them as soon as manifest.js is imported:

export const fallback = [c[0](), c[1]()];

Is there a reason why fallback needs to call those functions immediately? If the import was delayed until they are actually needed (in the _load_error function of src/runtime/client/renderer.js), would that cause any issues?

@rmunn
Copy link
Contributor

rmunn commented Aug 10, 2021

I've traced the origin of the export const fallback = [c[0](). c[1]()] line to #893 (where the name was components rather than c), and as far as I can tell there's no reason why those two components need to be imported immediately, so I tried this:

diff --git a/packages/kit/src/core/create_app/index.js b/packages/kit/src/core/create_app/index.js
index ea84e023..b5a1137c 100644
--- a/packages/kit/src/core/create_app/index.js
+++ b/packages/kit/src/core/create_app/index.js
@@ -102,7 +102,7 @@ function generate_client_manifest(manifest_data, base) {
 
 		export const routes = ${routes};
 
-		export const fallback = [c[0](), c[1]()];
+		export const fallback = [c[0], c[1]];
 	`);
 }
 
diff --git a/packages/kit/src/runtime/client/renderer.js b/packages/kit/src/runtime/client/renderer.js
index 4b1fdd10..7d9cbfb1 100644
--- a/packages/kit/src/runtime/client/renderer.js
+++ b/packages/kit/src/runtime/client/renderer.js
@@ -60,7 +60,7 @@ function initial_fetch(resource, opts) {
 export class Renderer {
 	/** @param {{
 	 *   Root: CSRComponent;
-	 *   fallback: [CSRComponent, CSRComponent];
+	 *   fallback: [() => CSRComponent, () => CSRComponent];
 	 *   target: Node;
 	 *   session: any;
 	 *   host: string;
@@ -708,7 +708,7 @@ export class Renderer {
 		};
 
 		const node = await this._load_node({
-			module: await this.fallback[0],
+			module: await this.fallback[0](),
 			page,
 			context: {}
 		});
@@ -718,7 +718,7 @@ export class Renderer {
 			await this._load_node({
 				status,
 				error,
-				module: await this.fallback[1],
+				module: await this.fallback[1](),
 				page,
 				context: (node && node.loaded && node.loaded.context) || {}
 			})

As far as I can tell that works (running pnpm test didn't turn up any failures). I don't have time to turn this into a proper PR with a proper test to exercise this scenario (that fails before the code change is applied and passes after it's applied), but perhaps this will help someone else create a PR to solve the issue.

@anudeepreddy
Copy link

I have tried the same and I can concur that every test was passing before and after the changes were made. But I was waiting to see if that was the right approach before raising a PR. Should I take this up and raise a PR?

@rmunn
Copy link
Contributor

rmunn commented Aug 11, 2021

... as far as I can tell there's no reason why those two components need to be imported immediately ...

Turns out there is a reason. According to #1356 (comment) it's deliberate to have the fallback components (the root layout and error components) imported during initial load, even if they're not used, precisely because they are fallback components. If something goes wrong in the process of loading a page, one of the likely reasons why it went wrong is a network failure, in which case loading the error components will fail too if they aren't already in the user's browser. That is why the root layout and error components are imported as soon as possible, so that they will be in memory in the user's browser and available to use without any further network traffic. Which means that if the network goes down five seconds after the user loads your site's first page, there's at least something that Svelte-Kit can display to inform them that the site isn't working right.

Therefore, this behavior is unlikely to change, and the only real fix to this issue is going to be to add a note to the documentation saying "Don't put side effects in the root layout module script, as they might be run twice, and/or run when you don't expect them to run."

@jsprog
Copy link
Author

jsprog commented Aug 11, 2021

@rmunn, This issue is not about not keeping src/routes/__layout.svelte with other resets and has nothing to do with #1356.

Could you at least explain these:

  • Why the additional log is only appearing when we update the root layout, until we restart the development server.
  • Why the additional log is coming from __layout.svelte instead of __layout.svelte? [sm]

@anudeepreddy
Copy link

@jsprog @rmunn help me understand this better. If I have imports of same module in two different places, will it be fetched twice? Or will vite play any role in this?

@jsprog
Copy link
Author

jsprog commented Aug 11, 2021

@jsprog @rmunn help me understand this better. If I have imports of same module in two different places, will it be fetched twice? Or will vite play any role in this?

I don't understand your question very well without the exact context. I'm not sure if you're referring to the abnormal log mentioned in this issue, or my text about app level imports mentioned in #2169

If you inspect the pictures above, you'll notice that all logs are from __layout.svelte? [sm] except the abnormal one which is from __layout.svelte. and another abnormality is that it only appears after updating __layout.svelte, and this trigger real browser's reload, and It's won't let go until you restart the development server.

If you're asking if an import could be evaluated twice during development, I already experienced this not a long time ago server side when trying to create a middleware for realtime communication. At first, I imported it from hooks.js and imported it again from svelte.config.js and used it there, to later find out the timer inside the middleware is logging twice. I didn't delved deep into details, but I suspect that two scripts were running in parallel and sharing the same process and console. but I don't understand why they were sharing the same global here #887 (comment). I can recreate an example for this on your request, but at least, let's keep this issue clean and focused.

@anudeepreddy
Copy link

So let me try to explain what I observed, I have disabled JS source maps in chrome dev tools to avoid any confusion in viewing the source files. Let me break this down into two parts:

  • one before force updating the __layout (only one log statement is printed).
  • And the other after force updating the __layout (this is when we start seeing two log statements).

Index Source
index source

Before

When we see the network calls made, we can see __layout.svelte being imported only once. And it is evident from the screenshot below that the import statement in manifest.js triggered this fetch.

before_network

After

Now after force updating the __layout.svelte and on browser reload we see that it is fetched twice. (see screenshot below)

after_network1
after_network2

It is fetched once from the (index) (see index source above) which contains import statement for __layout.svelte and the second fetch is from the manifest.js. And the console statement is run twice.

after_console

And you can see from the above screenshot that the second console.log comes from the __layout.svelte imported in manifest.js.

So back to the question I was asking before, There are two import statements for __layout.svelte in both cases (before and after) why is it that the first time we see it fetching once where as twice in the second case?

@jsprog
Copy link
Author

jsprog commented Aug 12, 2021

@anudeepreddy, I started to receive similar logs as yours (with query strings) after using an updated chrome browser.

Trying to understand how the development server is pushing the HMR updates to browser, I created a new file src/routes/foo.svelte and tested with route /foo. I already confirmed that the browser is always loading/reloading foo.svelte without the query string, and receiving hmr updates by appending a query string.

<!-- src/routes/foo.svelte -->
<div>foo</div>

So back to the question I was asking before, There are two import statements for __layout.svelte in both cases (before and after) why is it that the first time we see it fetching once where as twice in the second case?

You already discovered what's causing this issue. In the second case, one of the imports is appending a query string with a timestamp, causing the two imports to be evaluated independently as two different modules.

@benmccann benmccann changed the title Code inside the Module context for top level __layout.svelte may run twice after browser reload __layout.svelte may run twice after browser reload in dev Aug 12, 2021
@benmccann benmccann added p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc. and removed p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. labels Aug 12, 2021
@benmccann benmccann removed this from the 1.0 milestone Aug 12, 2021
@JulianvonGebhardi
Copy link

I have the same behavior while running dev mode but the layout gets only reinitialized (a second time) after the first change of navigation. Lucky I found this issue...I was getting mad since I thought I am causing the problem somehow. And i just realized it only happened in dev mode.

@benbesen
Copy link

benbesen commented Feb 1, 2022

I've been struggling with the same issue. Honestly I think it is quite an important bug, which really influences my productivity with Sveltekit as every item or component that gets initialized globally in __layout, is added multiple times. I have this kind of setup in more then 50% of my projects and to really check if the site is working I always need to build it which is quite annoying. If anybody knows a workaround for development i would be very thankful.

@onderbakirtas
Copy link

This issue happens to me as well. One of a few issues that makes SvelteKit is not that mature enough (this is stated in docs). Thanks for the comments above that made me understand the problem. Honestly I think this is a deal breaker for v1.0.

@Bandit
Copy link

Bandit commented Apr 21, 2022

In my experience this issue is caused by prefetch. Wish there was a way to disable prefetching in dev mode

EDIT: I just ran npx browserslist@latest --update-db and it fixed my dev server (temporarily) - maybe it's some sort of package caching issue?

@Rich-Harris Rich-Harris added the bug Something isn't working label Apr 26, 2022
@Rich-Harris Rich-Harris added this to the 1.0 milestone Apr 26, 2022
@TheophileMot
Copy link

TheophileMot commented Apr 28, 2022

@Bandit Thanks, that seems to have fixed a similar issue I was facing. (Nope, it's still firing twice.) When you say that the fix was temporary, do you mean that it broke again?

@Bandit
Copy link

Bandit commented Apr 28, 2022

Yeah it seems that doing something that restarts the server fixes the problem temporarily. You can simulate this by CMD+C'ing the server and starting it again and the problem goes away for a while.

@rajatbarman
Copy link

Same bug here :)
It's understandable as the kit hasn't even hit 1.0, loving the API and dev experience even in beta :)

@Rich-Harris
Copy link
Member

Should be fixed by #4891

@Rich-Harris Rich-Harris mentioned this issue May 12, 2022
5 tasks
@antidiestro
Copy link

antidiestro commented May 19, 2022

This is really annoying. Since I only have one element in my __layout.svelte file, I solved this temporarily by adding a data-layout attribute to said root element, checking for a duplicate on mount and removing it if necessary. Don't know if this is the best solution but it's working for me at the moment.

EDIT: Just realized that components inside the layout can also display the same behavior. Now I don't have duplicate layouts, but I do have two views at the same time. It's really unsustainable to develop with this bug.

<script>
  import { onMount, tick } from 'svelte';

  onMount(async () => {
    await tick();
    if (document.querySelectorAll('[data-layout]').length > 1) {
      document.querySelector('[data-layout]').remove();
    }
  });
</script>

<div data-layout="true">
  <slot />
</div>

@Bandit
Copy link

Bandit commented May 19, 2022

@ReneMoraales wish I'd thought of doing that 9 months ago! ha ha

@Egor-Demeshko
Copy link

Egor-Demeshko commented Sep 2, 2024

This is really annoying. Since I only have one element in my __layout.svelte file, I solved this temporarily by adding a data-layout attribute to said root element, checking for a duplicate on mount and removing it if necessary. Don't know if this is the best solution but it's working for me at the moment.

EDIT: Just realized that components inside the layout can also display the same behavior. Now I don't have duplicate layouts, but I do have two views at the same time. It's really unsustainable to develop with this bug.

<script>
  import { onMount, tick } from 'svelte';

  onMount(async () => {
    await tick();
    if (document.querySelectorAll('[data-layout]').length > 1) {
      document.querySelector('[data-layout]').remove();
    }
  });
</script>

<div data-layout="true">
  <slot />
</div>

faced the same issue, i have two part of view rendered twice.
if i wrap in div bigger part of the same view rendered twice.
i have page that rendered different country information from database but only on one country this effect can be encountered.

i can provide picture if someone will be intrested.

effect can be achieved only on refreshing page with refresh button and if we go to page through advertising company in search)))

@Egor-Demeshko
Copy link

Egor-Demeshko commented Sep 2, 2024

изображение

this helps, but now have some problems related animation)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.