denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
94.08k stars 5.23k forks source link

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

Closed happyhunter7 closed 4 days ago

happyhunter7 commented 4 days ago

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

happyhunter7 commented 4 days ago

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 commented 4 days ago

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

nathanwhit commented 4 days ago

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 commented 4 days ago

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.

happyhunter7 commented 4 days ago

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

happyhunter7 commented 4 days ago

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

happyhunter7 commented 4 days ago

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 commented 4 days ago

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 commented 4 days ago

Closing in favor of https://github.com/denoland/deno/issues/8327

happyhunter7 commented 3 days ago

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 commented 3 days ago

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?

happyhunter7 commented 3 days ago

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

happyhunter7 commented 3 days ago

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

happyhunter7 commented 3 days ago

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 commented 3 days ago

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.

happyhunter7 commented 3 days ago

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

happyhunter7 commented 3 days ago

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

happyhunter7 commented 3 days ago

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 commented 3 days ago

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

deno upgrade --rc
happyhunter7 commented 3 days ago

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)
happyhunter7 commented 3 days ago
 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

happyhunter7 commented 3 days ago

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 commented 3 days ago

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 commented 3 days ago

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

happyhunter7 commented 3 days ago
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
}
happyhunter7 commented 3 days ago

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

happyhunter7 commented 3 days ago

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

happyhunter7 commented 3 days ago

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

happyhunter7 commented 3 days ago

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

bartlomieju commented 3 days ago

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.

happyhunter7 commented 3 days ago

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

happyhunter7 commented 3 days ago

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