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

Dynamic imports get cached and there is no way to get the fresh version of it #25742

Closed
gherciu opened this issue Sep 19, 2024 · 32 comments
Closed
Labels
working as designed this is working as intended

Comments

@gherciu
Copy link

gherciu commented Sep 19, 2024

Version: Deno 1.46

The bug:

file a.ts

setInterval(() => {
   import(`./b.ts?id=${Date.now()}`).then(console.log)
}, 1000)

file b.ts

import c from './c.ts'

export default c

file c.ts

const myString = 'abc'

export default myString

Then run the deno run ./a.ts

Now this will console log the result of dynamic import, and THIS IS IMPORTANT the dynamic import has a query param which means DENO should import a fresh version each time and not from cache

But if while this script runs you go to c.ts and change the string manually the console log will not change, since C inside B is cached which is supper bad since I have a query param in B and I expect a fresh version of it and it's child imports also should be not cached for this exact import

Well that obsiously sucks, because that literally means you can't load user code since only first file will be Fresh/last version everything inside again is cached

@gherciu
Copy link
Author

gherciu commented Sep 19, 2024

And Yes this is a important use case and I'll tell why

Imagine you build Next.js in Deno
You literally can't import user code on server and do server side rendering

Imagien you have a file named _app.tsx and you import it dynamically and render, then something changes inside while the server is still running, now on next serverSideRendering request it will give you the same result even if the jsx of some child import changed
There is literally no way the moment to do fresh imports in deno, and build big tools like Next js

Or is there some alternative to require??
Maybe if I build a JS lib which uses require and then import in deno would that fix ? is that even possible


Tested it you can't even import a npm module which uses require)

Basically as I understand it means you have to terminate the process and restart it
which kinda suck if running in docker or similar

maybe a script that terminates another script s needed something like pm2(

@marvinhagemeister marvinhagemeister added the bug Something isn't working correctly label Sep 19, 2024
@marvinhagemeister marvinhagemeister added this to the 2.0.0 milestone Sep 19, 2024
@marvinhagemeister
Copy link
Contributor

marvinhagemeister commented Sep 19, 2024

This is a regression and definitely something that should work. Haven't verified this, but it could be related to denoland/deno_core#906 nvm, it's reproducible in 1.46.3

@nathanwhit
Copy link
Member

nathanwhit commented Sep 19, 2024

I don't believe this ever worked. From testing, this behavior is consistent all the way back to 1.7.0 (the oldest release deno upgrade works with).


As noted:

But if while this script runs you go to c.ts and change the string manually the console log will not change, since C inside B is cached which is supper bad since I have a query param in B and I expect a fresh version of it and it's child imports also should be not cached for this exact import

To highlight this, the crucial bit here is that we would have to reload all children of a given module. While we do reload b.ts (because of the query string), that doesn't mean we reload c.ts - its specifier is the same and resolves to the same location.

@marvinhagemeister marvinhagemeister removed the bug Something isn't working correctly label Sep 19, 2024
@marvinhagemeister marvinhagemeister removed this from the 2.0.0 milestone Sep 19, 2024
@marvinhagemeister
Copy link
Contributor

Ohh right, I missed that bit. Just checked all the other runtimes and they only refresh the module with the query param, not any further children of said module. This works in Deno as expected when I change something in b.ts.

@marvinhagemeister marvinhagemeister added the question a question about the use of Deno label Sep 19, 2024
@gherciu
Copy link
Author

gherciu commented Sep 19, 2024

So is there a change that it will be fixed soon?
I'm trying to migrate a tool to Deno and it is a big blocker

What I had to do for the moment is to implement something similar to PM2, a process that controll other process so that I lose the cache just by killing the child process and restarting

This is obviously a workaround and I would want the dynamic import to work correctly, for a better developer experience

Also this blocks migration of some tools to deno which relly on dynamic require

@gherciu
Copy link
Author

gherciu commented Sep 19, 2024

Can we move this to BUG label
Since indeed there is no sense now in query string on a dynamic import, it just makes the developers feel like it will import a fresh version of everything which is not True at all, it is indeed a bug
A fresh module can't work with cached childrens, that's the whole purpose of adding query string to a import
cc @marvinhagemeister @nathanwhit @lucacasonato

@gherciu
Copy link
Author

gherciu commented Sep 19, 2024

If I add specially ? to the import then obviously I do not need the child imports to be cached
Maybe there is a chance to add some option to the dynamic import like uncached , and when this option is passed it would import everything inside without looking at the cache, and by default it is false which will not break someones current code

but still others will have the possibility to import uncached modules

In Node there are NPM packages that can do that but uses require.cache. but in deno there is no solution from what I can see

@littledivy littledivy added bug Something isn't working correctly working as designed this is working as intended and removed question a question about the use of Deno bug Something isn't working correctly labels Sep 20, 2024
@littledivy
Copy link
Member

This is working as intended as per spec. Cache busting with ? is expected to only update the module not it's own dependency graph.

I think your problem can be solved by exposing a module loader API. See #8327 . For now, you have to manually cache bust all modules in the graph to achieve.

@littledivy
Copy link
Member

Closing in favor of #8327

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

Lol)
Imagine loading a page and having a button from other page loaded previously, what a stupidity
In node you can remove the cache, in deno this task is sitting for years, why we close if the one opened no one takes any action for years
4 years ago was opened, and everyone knows this is a necessity to build developer tools like Next js

Working as designed dones't mean that the design is correct
Having only first level fresh and inherited cached just creates bugs, that's a bad design

In tools like esbuild this works correct because every import is suffixed, and they can do that because is a transpiler
In Deno you can't compile the code because that is it purpose to be able to run TS without compiling or suffixing imports
And you do not have power over the inherited import, as what you have in esbuild for example

You apply a JS mindset where you can compile from TS to js and have power over the resulted code....... to Deno, is that a correct design?

@marvinhagemeister
Copy link
Contributor

marvinhagemeister commented Sep 20, 2024

The behaviour in Deno matches the behaviour of all other runtimes. What you're asking for is either a loader API, or what is more likely the proper solution is some form of transpilation like nextjs, vite, esbuild and all others are doing. Since Deno sees itself as a runtime, not a bundler, moving the cache busting logic into a bundler is more appropriate.

That said there is also a --watch-hmr flag that reloads modules automatically in Deno when a file changes on disk and patches modules in the running process if possible. Maybe that's what you're looking for?

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

What we need is a implimentation of require.cache, where we can delete something from cache at least, same as in Node, and access to the child imports

Or some new method like importUncached which will import the module and also it's child in a uncached manier

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

I've tried --watch-hmr
It just restarts the whole program, same as you would spawn a child process and respawn when needed
At least with a spawn you have controll when to respawn but with --watch-hmr you can't tell when

Also if you have to notify a browser for example once something changed in _app.tsx you can't do that since the program is restarted and you cant send any event to browser to reload it as well
So if you have a program that compiles Browser code and have to notify it when changes you can't do that, because the whole Deno process would start prior and kill any socket connections or SSE handler

And yes Browser code is imported in Deno because you need to do server side rendering and once something changes in client code deno also will restart it's whole process

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

And yes to your point the behavior indeed matches, but in other runtimes you have other tools that can remove the cache in Deno NO that's the difference

@bartlomieju
Copy link
Member

What we need is a implimentation of require.cache, where we can delete something from cache at least, same as in Node, and access to the child imports

This is available in Deno:

import { createRequire } from "node:module";

const require = createRequire(import.meta.url);

console.log(require)
deno run -A main.js
[Function: require] {
  resolve: [Function: resolve] { paths: [Function: paths] },
  main: null,
  extensions: [Object: null prototype] {
    ".js": [Function (anonymous)],
    ".mjs": [Function (anonymous)],
    ".json": [Function (anonymous)],
    ".node": [Function (anonymous)]
  },
  cache: [Object: null prototype] {}
}

You can clear the cache if you want.

I believe part of the confusion stems from the fact that require (CommonJS) works completely different than ESM. In ESM there's no way to "unload" a module a load it again. The closest thing is to do the cache busting using query params. Tools like Vite handle that internally IIRC - ie. Vite adds hash or timestamp to each import. For other tools like Next.js even though they use import ... from ... syntax they actually transpile it down to CommonJS, which as you mentioned can just use require.cache and clear it to "reload" the module.

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

Ok let me try this then
I'll try to implement with this something similar to https://www.npmjs.com/package/import-fresh
maybe that would work for my use case

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

Actually no it is not workig)
Because of this

Uncaught (in promise) Error: require() of ES Module

So aparently you can't uncache a ES module once loaded
And for import there is no method like import.cache same as require.cache

Which means once a Deno TS file is loaded then tehre is no way to uncache it's children imports
Only way is to restart the process, which for a application is not possible, because if you run a server this would kill all connections

And this is a real use case, I can give another example
Imagine you were building Wordpress but in Deno
Once a theme changes in runtime, you have to restart the server and kill all user conenctions, which kinda sucks
Otherwise the server side rendering of a template would give same result even if theme changed

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

hmmmm

https://bun.sh/docs/runtime/modules#:~:text=In%20Bun's%20JavaScript%20runtime%2C%20require,exports%20object%20(as%20in%20Node.
Seems that in Bun they have such a feature, and require can import esm and remove cache, I'll try to check what are the implications to move to Bun
Thanks for your time

@littledivy
Copy link
Member

littledivy commented Sep 20, 2024

require(ESM) works in Deno too, it was released in the 2.0 RC release:

deno upgrade --rc

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

In RC this import throws a error

error: Uncaught (in promise) SyntaxError: Cannot use import statement outside a module
    at Object.evalContext (ext:core/01_core.js:680:31)
    at wrapSafe (node:module:712:25)
    at Module._compile (node:module:729:23)

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

 const require = nodeModule.createRequire(import.meta.url)

    delete require.cache[require.resolve(path)]
    importResult = require(path) as T

The result is kinda the same as in 1.46) but a different error, Require doesn't seem to be able to import a ES module
In my case I try to import a TS file a Deno TS file which s a ES module

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

From what I can see in Bun they can import TS with require

const { baz } = require("./baz.tsx");

https://bun.sh/docs/runtime/modules#:~:text=In%20Bun's%20JavaScript%20runtime%2C%20require,exports%20object%20(as%20in%20Node.

So maybe the Deno impliemntation in RC is buggy, Or I do something incorrect?
Because I use the createRequire? maybe just require is in global and I do not have to create it? No it is not, feels like a bug to me

@bartlomieju
Copy link
Member

In RC this import throws a error

error: Uncaught (in promise) SyntaxError: Cannot use import statement outside a module
    at Object.evalContext (ext:core/01_core.js:680:31)
    at wrapSafe (node:module:712:25)
    at Module._compile (node:module:729:23)

Can you share the code that causes this error?

@bartlomieju
Copy link
Member

This works correctly for me:

// bar.ts
export function greet() {
  return "Hello!"
}
// some_file.ts
import { greet } from "./bar.ts";

export default {
  a: "b",
  foobar: "bar",
  greet,
}
// foo.js
import { createRequire } from "node:module";
const require = createRequire(import.meta.url);
const a = require("./some_file.ts");
console.log(a)
deno run -A foo.js
[Module: null prototype] {
  default: { a: "b", foobar: "bar", greet: [Function: greet] }
}

I'm using Deno v2.0.0-rc.4

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

export const uncachedDynamicImport = async <T>(path: string): Promise<T> => {
  let importResult: T

  if (Deno.build.os === 'windows') {
    const importId = Date.now()
    importResult = (await import(`${path}?import_id=${importId}`)) as T
  } else {
    const nodeModuleImport = await import('node:module')
    const nodeModule = nodeModuleImport.default
    const require = nodeModule.createRequire(import.meta.url)

// DELETING the cache, in future I'll implement child cache deletion as well same as in npm util import-fresh
    delete require.cache[require.resolve(path)]
    importResult = require(path) as T
  }

  return importResult
}

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

So I basically try to build a util that uses require instead of import so that I can delete the cache for a path
and also I switched to deno 2 rc and the error above is thrown

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

And then the files that this utility try to load are absolute paths to TSX files and it doesn't work

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

It basically says that I can't use import outside a module, but how do I convert it to a module)) if it is already a TS file and is a module

Cannot use import statement outside a module
    at Object.evalContext (ext:core/01_core.js:680:31)
    at wrapSafe (node:module:712:25)
    at Module._compile (node:module:729:23)
    at Object.Module._extensions..js (node:module:767:10)
    at Module.load (node:module:665:32)
    at Function.Module._load (node:module:537:12)

Or maybe I should split this util in 2 files one for web and one for server? maybe when it see both require and import in same file it complains? but why I would expect it to work anyway

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

All the files are btw TS or TSX no JS at all

@bartlomieju
Copy link
Member

Sorry but without a full reproduction I can't really say what's going wrong. If you have a repo that can be tested then I can take a look.

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

Ok so I tested all 3
Node, Bun, Deno
And seems that if you use import then you can't really mix it with require)) because anyway at the end at some step you'll get some error
And yes you cant use require to import something which uses import, because then it says
Cannot use import statement outside a module
And that's in all 3 languages)
And even if you can like in Bun, it is just buggy require.cache is always empty and they would not fix it soon

Seems that indeed only a Module Loader can help here
But that task in Deno is staying open for almost 4 years now

@gherciu
Copy link
Author

gherciu commented Sep 20, 2024

That being said I can 100% guarantee that a library like Wordpress which allows to install plugins and themes at runtime and change their code at runtime, is imposible to be built in JS world
Actually is possible but only with CommonJS and no TS at all and only using Require everywhere which no user would install your library, because nowadays everyone moves to TS)

We'll I guess then it becomes a Feature Request, and if Deno would impelemet a uncachedImport util then deno may be the first JS like language to have frameworks like Wordpress built on it with THE EXACT same DX and UX as there and maybe gain same popularity

@bartlomieju Thanks for your time, I'll open a feature request for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
working as designed this is working as intended
Projects
None yet
Development

No branches or pull requests

5 participants