__layout.svelte may run twice after browser reload in dev

[jsprog]

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

After the update (notice the abnormal additional log changing positions)

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.

[vwkd]

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]

#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]

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]

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]

... 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]

@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]

@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]

@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]

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

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.

After

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

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.

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]

@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.