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

Add a way to dynamically import the same module multiple times #6946

Closed
lem0nify opened this issue Aug 3, 2020 · 21 comments
Closed

Add a way to dynamically import the same module multiple times #6946

lem0nify opened this issue Aug 3, 2020 · 21 comments
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)

Comments

@lem0nify
Copy link

lem0nify commented Aug 3, 2020

Deno doesn't support require but there is one thing where require wins against ES6 imports:

In Node.js we can do

delete require.cache[require.resolve(somePath)];

and delete required file from cache to load it again after it get changed in runtime. With imports we can't do like this. Even if we use dymanic imports (await import("./module.ts")) twice, we both times get the module Deno loaded first time.

There is a workaround about it. We can add a query parameter to file path like this:

mod = await import(`./mod.ts?foo=${anyRandomlyGeneratedString}`);

and it will be loaded again and we get a new its version. But the problem is that BOTH versions of this module will exist in the cache, I suppose. That's not cool.

Why do we even have to store dynamically imported modules in cache? Maybe better make them uncached and garbage-collected and make it possible to import the same module multiple times and each time get the current module content without any ugly workarounds? Otherwise it looks like dynamic imports are just called dynamic but still work in a static way.

@Soremwar
Copy link
Contributor

Soremwar commented Aug 3, 2020

This is a requirement to make Deno's security system work, since analysis is made on compile time, then modules cached so they can be immutable, hence not breaking the sandbox in the process.

I would argue that altering the content of a module in runtime is not only insecure but also pointless, what possible use case would you have for something like this that can't be solved by restarting the Deno process?

@lem0nify
Copy link
Author

lem0nify commented Aug 3, 2020

@Soremwar

what possible use case would you have for something like this that can't be solved by restarting the Deno process

A module-based extendable proxy server with packet spoofing. If you want to change logic of some module, you have to restart a server and a program which is communicating though it.

Or anything else that should run constantly but give a user freedom to extend its logic.

@kitsonk
Copy link
Contributor

kitsonk commented Aug 3, 2020

Why do we even have to store dynamically imported modules in cache?

Because that is very similar to the way browsers work. Clearly dynamic implies something to you that it doesn't actually mean to imply, it means "allow me to conditionally run this code in the current execution context", but all the rules to how modules are fetched, compiled and imported apply whether it be static or dynamic.

Ultimately this is a duplicate of #5548 and #6694

@nayeemrmn
Copy link
Collaborator

There is a workaround about it. We can add a query parameter to file path like this:

mod = await import(`./mod.ts?foo=${anyRandomlyGeneratedString}`);

and it will be loaded again and we get a new its version. But the problem is that BOTH versions of this module will exist in the cache, I suppose. That's not cool.

@lem0nify Instead of changing the query string, you can change the hash. When you do this the new cached code replaces the old. However it will not cause remote dependencies to be re-downloaded. Only recompiled (if the source is local and changed) and re-executed. Does that meet your needs?

@lem0nify
Copy link
Author

lem0nify commented Aug 3, 2020

@nayeemrmn No, it doesn't. Did you properly read my issue? I ask about module reloading in runtime, without necessity to even restart a process.

UPD. What does your reaction mean? Did I wrongly understand your answer? Is it possible to recache module and reload it in runtime?

@kitsonk
Copy link
Contributor

kitsonk commented Aug 4, 2020

Basically what you are asking for is hot module reloading... That is problematic in a lot of ways. ES Modules and their exports are immutable by design. As stated in #5548 we should find better ways to do this to support things like development. There is discussion in #6694 about this as well.

Instead of asking for a specific feature, what use case are you trying to solve?

@lem0nify
Copy link
Author

lem0nify commented Aug 5, 2020

@kitsonk I've mentioned before, in my answer to Soremwar, but I can explain more detailed.

There is a project called tera-toolbox. It is framework for modding an online-game called TERA Online through proxifying game traffic and packet hooking. It supports module hot reloading but it is written in Nodejs and uses classic require for module (re)loading. Sometimes it is problematic to write a working module right away and it's very comfortable that you don't need to restart tera-toolbox and TERA to make fixes and see how they work because game restart may take few minutes.

My goal is to write a similar framework for a different online-game (I won't mention which game exactly if you don't mind) but I like deno much more than nodejs in many other aspects so I want to switch to it from node.

@kevinkassimo
Copy link
Contributor

kevinkassimo commented Aug 5, 2020

Based on my understanding this is one core design related to ES6 imports and while theoretically we can do hacks to force the runtime to "forget" about a module and its dependencies (since we have control to host_import_module_dynamically_callback and full module resolution logic), it's beyond standards and might cause future regrets.

In the meantime, we still have a require polyfill in std/node/module.ts, so you can probably use it just like what you would do in Node.js. (with restriction that you either have to rely on CommonJS modules with it or use dynamic imports only for ES6 modules. Also you have to explicitly grant read permission to these module files as they are treated more like data than scripts)

@lem0nify
Copy link
Author

lem0nify commented Aug 5, 2020

I can't understand guys why are you so worried about compatibility with standards and V8 design dogmas. V8 is designed to work in browsers on client side with a bunch of restrictions. If we've already ripped V8 out of its intended environment to use JS on the server side, what's wrong with making some changes to its set of rules and opening up some additional features useful for server and desktop development? Especially considering that these changes contradict the standard only in terms of implementation, not in terms of use.

@Soremwar
Copy link
Contributor

Soremwar commented Aug 5, 2020

@lem0nify Node didn't follow the ECMA standards either. Now every update they have to make is a pain in the ass for the devs

Why do you think ES modules are still experimental 5 years later?

@kitsonk
Copy link
Contributor

kitsonk commented Aug 5, 2020

If we've already ripped V8 out of its intended environment to use JS on the server side, what's wrong with making some changes to its set of rules

https://deno.land/manual#goals

@kevinkassimo
Copy link
Contributor

kevinkassimo commented Aug 5, 2020

@lem0nify Standard compliance is a core idea of the project from almost Day 1, as we want to maximize browser compatibility for many common utility modules. IMO you either accept the standard and follow the rules, or throw the standards out of the window and do your own thing (and in that case, we offer the option of Node-like require in standard modules -- which is not part of the core binary). A middle ground between the two, while sound tempting, is likely to eventually result in some kind of a turmoil in the long run: small bits of complexity added to support a non-standard feature while trying to maintain standard compliance would gradually grow and become huge burden and legacy. If this general need gains enough traction, the standards committee would react -- which often will provide deeper insight and more comprehensive solution than the hacks we can do to just support a single specific use case

@lem0nify
Copy link
Author

lem0nify commented Aug 5, 2020

@Soremwar Because they designed something not in the best way. And the best way doesn't have to be full compliance with the standards, I think.

@kitsonk Are you talking specifically about the following goal?

Browser compatible: The subset of Deno programs which are written completely in JavaScript and do not use the global Deno namespace (or feature test for it), ought to also be able to be run in a modern web browser without change.

Do you at least in theory imagine such program? A library, maybe, but not an application. But the library targeting to compatibility with web browsers won't use dynamic imports, I guess. And also, how about import.meta.main? Is it supported by modern web browsers? : ^)

Okay, even considering these goals, why not make some cache cleaning tools inside Deno namespace?

@kevinkassimo Okay, sure, it sounds convincing. But I still call on you, smart guys who contribute to Deno to think about ways to implement module hot reloading without need to use NodeJS compatibility API (which forces me to write modules in CommonJS style). Maybe a good and painless compromisse can be found.

@timreichen
Copy link
Contributor

I think removing a cached file is doable without chaning any specs or treatment.
I use the cache system in a project and therefore created this module. It provides the functionality of deno cache and resolving an url to the cached file path.

Not tested: You could theoretically call Deno.remove on the file path and its json file to remove it from cache, then reimporting it.

@lem0nify
Copy link
Author

lem0nify commented Aug 8, 2020

@timreichen Sad but no. Looks like deno process remembers content of every imported .ts file and transpiles to js its RAM-stored version without reading the new content of the file. I've tried to remove cached files by hand and change module, but on second import it just created .ts.js in cache again with its old content.

@nayeemrmn
Copy link
Collaborator

@lem0nify

let i = 0;

// Initial import:
let mod = await import(`./mod.ts#${i++}`);

// Reload:
mod = await import(`./mod.ts#${i++}`);

// Reload:
mod = await import(`./mod.ts#${i++}`);

@lem0nify
Copy link
Author

lem0nify commented Aug 9, 2020

@nayeemrmn Sure, but:

  1. Copy of every version of module will persist in memory and cause memory leak.
  2. We also need //@ts-ignore before every such import, because of TS2307 [ERROR]: Cannot find module './mod.ts#1' or its corresponding type declarations.

@nayeemrmn
Copy link
Collaborator

  1. Copy of every version of module will persist in memory and cause memory leak.

Won't it just get garbage collected like anything?

2. We also need //@ts-ignore before every such import, because of TS2307 [ERROR]: Cannot find module './mod.ts#1' or its corresponding type declarations.

Yeah that's a bug.

@lem0nify
Copy link
Author

lem0nify commented Aug 9, 2020

@nayeemrmn Oh, wait. No, we don't need //@ts-ignore when are using template strings. I've just tried with flat "./mod.ts" and "./mod.ts#1".

Won't it just get garbage collected like anything?

As far as I understand, no. Maybe, JavaScript objects will be GC'ed but not deno process' RAM cache I've told two messages before about.

@lucacasonato lucacasonato added cli related to cli/ dir suggestion suggestions for new features (yet to be agreed) labels Aug 17, 2020
@guiprav
Copy link

guiprav commented Oct 6, 2020

@lem0nify, I think if ES module caches aren't implemented using GC-friendly "weak refs" (see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakRef), that's a potential bug/enhancement to V8/whatever actually implements the module cache. If they do, OTOH, and your code doesn't hold any strong refs to the old module, it should be GC'ed just fine.

But even if your modules leak, for your particular use-case that should be a total non-issue. Unless the modules are allocating many MiBs of memory and holding strong refs to all that memory, you'd have to reload your module thousands, maybe millions of times for it to cause any significant memory usage problems.

My two cents is that hash-based cache-busting should suit your use-case and most others pretty well. It's pretty ugly, but that's probably a simpler solution than figuring how to safely expose the module cache like Node does. Also, it's a widely known cache-busting technique, so it's not like you're doing anything too exotic there.

Like some have pointed out, one of Deno's goals is to keep the solid sandbox characteristics of browser JS and APIs, so imo this reluctance to even give it much thought is pretty understandable. I don't develop Deno but I think the idea does raise some red flags :P

Kudos for your great taste in waifu.

@kitsonk
Copy link
Contributor

kitsonk commented Nov 10, 2020

Lots of good conversation here. In the end, though, we are not going to "break" the way ES Modules are loaded in v8.

The way Node.js community is a module loader API and a "transparent" module hashing solution (e.g. quibble's ESM support). That is something that might work for Deno as well and have opened up #8327 to suggest that.

Because we won't directly consider this suggestion, I am respectfully closing in favour of #8327.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

8 participants