The new endpoint fetching via dynamic import leads to resource leakage, use alternative evaluation instead

[Tal500]

Describe the problem

After PR #6318, instead of fetching JSON response from the endpoint, Kit uses dynamic import directly to fetch server data.
Disclaimer: I'm on the party who thinks this decision is too unorthodox in general, but this isn't what I want to discuss here, since the deed is done, I rather discuss about some symptom of it and how can we fix this.

The problem starts when we wants to have an SPA experience which lasts for long time (e.g. in progressive web apps, but not only), so we don't expect the user to refresh after X number of interaction, at most.
By design, as far as I know, there is no way to de-allocate the resources of dynamic import of ES module. Probably the reason was because it was expected to have a limited number of modules you wish to load, and the dynamic support for import was added not for allowing you to import infinite amount of resources, but rather for allowing you to lazy/on-demand import your module for interactive performance balancing.

In the new method (via PR #6318), after every data loading, another module is imported, which means the browser keeps the module resource in the memory, and never releases it - and there is no way even to tell the browser to.

Describe the proposed solution

Since the fetched JS code is (and should) be very basic, like window.some_var = {}, I'm suggesting two alternative for the resourceful import:

1. Fetch the code from the server by the classic fetch(), and use the JS function eval() to evaluate the code.
2. Use a self-destructing script loading (like a classical JQuery-era techniques), something like:

new Promise((resolve, reject) => {
  const script = document.createElement('script');
  script.src = url;
  const remove = () => { document.head.removeChild(script) };
  script.onload = () => {
    remove();
    resolve();
  };
  script.onerror = () => {
    remove();
    reject(`Error while trying to inject <script src="${url}"> to the head element.`);
  };
  document.head.appendChild(script);
})

Option 1 for sure is best&secure memory-wise (and also simple for caching challenges), however I think that the evaluation of option 2 might be slightly faster.

Alternatives considered

No response

Importance

nice to have

Additional Information

I chose the importance to be "nice to have", but I think it's much more important for stability. This issue can be thought between feature and bug.

[dummdidumm]

AFAIK this was considered while designing this approach, and that the window method deallocates the module. Do you have concrete data that this happens?

[Tal500]

AFAIK this was considered while designing this approach, and that the window method deallocates the module. Do you have concrete data that this happens?

I saw no official technical claim about it, but I'm using only my logic here.
The browser probably keeps track on the imported modules, for the following reasons:

1. To not reimport an already imported module.
2. To display information and track the code for debugging purposes in the developer console.
3. For security?
4. Because the browser creators weren't focused on "automatically free dynamically imported module resources traces", since it wasn't there concern. As I said in 2, if they were doing it automatically it would also disrupt the developer console debugging/tracing ability.

If we really want to verify this, we can make a DoS style demo somehow and track the browser memory, but for me it's obvious that it will cause leakage. I just don't know how bad the leakage is, however every stable framework needs to have zero-leakage.

The alternative solutions I showed seems to be pretty easy comparing to a direct (very!) dynamic import.

Another disclaimer: I was developing in C++ for a long time, so this is why I'm sensitive to memory leakage. The motto/experience is that the memory always leaks unless proven otherwise.

[benmccann]

I think we'd need more than just a feeling that there might be something wrong. Yes, if you could make a demo that actually leaks memory that we can see using the Chrome resource monitor or similar then we could take this seriously.

[Rich-Harris]

To add some context: the reason we use this approach rather than either of the two proposed alternatives is that they will fail under common CSP constraints, unlike import

[Tal500]

I think we'd need more than just a feeling that there might be something wrong. Yes, if you could make a demo that actually leaks memory that we can see using the Chrome resource monitor or similar then we could take this seriously.

OK I did a simple demo. I had added the following code to the todo page:

import { onMount } from 'svelte';
onMount(async () => {
	for (let i = 1; true; ++i) {
		const url = `/todos/__data.js?__invalid=ny&__id=${i}`;
		await import(/* @vite-ignore */url);
	}
})

And memory capture, running in preview mode(after build, not in dev mode): (Warning, right to left text direction, and not English)
Before:

After 48 minutes:

It seems to not cause such a huge leak in memory, and I'm not sure how much of the added ~12MB is because of it.
I'm still convinced that the browser must track that the module was loaded somehow, otherwise it could have loaded the same module twice(and according to the standard it shouldn't). Probably it marks the already loaded modules simply in a hash map, and hopefully not wasting so much resources.

So as long as there will be no memory leak report in the future, I think we're all convinced it's probably fine to use import this way, at least in this sense.

Thanks!

[Tal500]

Update: This issue is really fixed now by #7177.