deno_core icon indicating copy to clipboard operation
deno_core copied to clipboard

Proposal: Enhance ModuleMap

Open CyanChanges opened this issue 6 months ago • 4 comments

Related: https://github.com/denoland/deno/issues/8327 Pull Request: #1146

Usage

Goal: Precise Handling HMR Including:

  • Tracking import dependency tree of a module (decide on what is needed to be reloaded)
  • Invalidate cached import, re-evaluate on updated content (preform module reload)

Monkey patch

  • Create Virtual Module
  • Alias name to another module

Alternative considered

  • Adding a suffix to import (.e.g ./some_module.ts?t=2333)
    • this module could import other modules (and other modules won't be re-evaluated in the case)
    • parts need to be reloaded is a dependency of the module (the module needs to be reloaded is not a module you import directly)

Just re-evaluate one entrypoint module will not work.

  • Use bundler to transform code
    • needs complex logic have to implement your own customized import(), require() learn how to use bundler API (Rollup, Webpack, Esbuild, etc.)
    • use Deno stop making sense Node.js you can run JS/TS in Node.js too If you want, you can use Node.js internal (like node:internal/modules/esm/module_job) to have similar things mentioned in the proposal Bun consider Bun supports require.cache for both ESM and CJS out of box. still missing a few but good enough for many cases

Bundler is complex and you need spend many more efforts using Bundler. You have the choice in other JS runtimes. So why shouldn't we have that?

Interface

Propose to allowing users to mutate ModuleMap, e.g.

  • set specifier to module id
  • delete the associated module id by specifier
  • alias specifier to another specifier
  • remove the alias mentioned above

Impl following to the ModuleMap struct


// get the SymbolicModule for `name`
pub fn get<Q>(
  &self,
  name: &Q,
  requested_module_type: impl AsRef<RequestedModuleType>,
) -> Option<SymbolicModule>
  where
    ModuleName: Borrow<Q>,
    Q: Eq + Hash + ?Sized;

// set the SymbolicModule for `name`
pub fn set(
    &mut self,
    name: ModuleName,
    symbolic_module: SymbolicModule,
    requested_module_type: impl AsRef<RequestedModuleType>,
  ) -> Option<SymbolicModule>;

// set `name` to be import to module namespace with module id of `id`
pub fn set_id(
  &self, 
  requested_module_type: RequestedModuleType,
  name: impl AsRef<str>, 
  id: ModuleId
) -> Option<SymbolicModule>;

// delete so import(`name`) will re-evaluate the module with new ModuleId
pub fn delete_id(
  &self, 
  requested_module_type: impl AsRef<RequestedModuleType>,
  name: impl AsRef<str>, 
  id: ModuleId
) -> Option<SymbolicModule>;

// alias so import(`name`) will have same effect of import(`alias`)
pub fn alias(
  &self, 
  requested_module_type: RequestedModuleType,
  name: impl AsRef<str>, 
  alias: impl AsRef<str>
) -> Option<SymbolicModule>; 

// readonly access specifier -> SymbolicModule map with specified type
pub fn with_map(
  &self,
  requested_module_type: impl AsRef<RequestedModuleType>,
  f: impl FnOnce(Option<&HashMap<ModuleName, SymbolicModule>>),
);

// API visibility changes

// from pub(crate) to pub
pub fn get_name_by_id(&self, id: ModuleId) -> Option<String>;

// from pub(crate) to pub
pub fn get_type_by_module(
  &self,
  global: &v8::Global<v8::Module>,
) -> Option<ModuleType>;

// from pub(crate) to pub
pub fn get_id<Q>(
  &self,
  name: &Q,
  requested_module_type: impl AsRef<RequestedModuleType>,
) -> Option<ModuleId>
  where
    ModuleName: Borrow<Q>,
    Q: Eq + Hash + ?Sized;

// from pub(crate) to pub
// allow you to track module imports
pub fn get_requested_modules(
    &self,
    id: ModuleId,
  ) -> Option<Vec<ModuleRequest>>

// from pub(crate) to pub
pub fn get_name_by_module(
    &self,
    global: &v8::Global<v8::Module>,
  ) -> OptionString>;

// from pub(crate) to pub
// also return &ModuleName instead of String to avoid Clone
pub fn get_name_by_id(&self, id: ModuleId) -> Option<&ModuleName>;

Added a associated function in JsRuntime, allowing users to acquire Rc<ModuleMap> in a op

/// SAFETY: 
/// try acquire module map with HandleScope from a Isolate 
/// that is not initialized with `deno_core::JsRuntime` will be undefined behavior
/// 
/// manipulating internal module may cause problem
pub unsafe fn module_map_from(scope: &mut HandleScope) -> Rc<ModuleMap> {
  JsRealm::module_map_from(scope)
}

CyanChanges avatar Jun 10 '25 12:06 CyanChanges

Corresponding PR for the issue ~~will be coming soon~~ is #1146

UPDATE: #1146

CyanChanges avatar Jun 10 '25 14:06 CyanChanges

I don't think this makes much sense for a module loader api. It doesn't need any ability to mutate existing entries in the map, only to intercept new requests. This can be done without any new methods on the moduleMap itself, you only need the ModuleLoader trait.

devsnek avatar Jun 10 '25 16:06 devsnek

I don't think this makes much sense for a module loader api. It doesn't need any ability to mutate existing entries in the map, only to intercept new requests. This can be done without any new methods on the moduleMap itself, you only need the ModuleLoader trait.

You need to mutate existing entries to do HMR, you need to invalidate the module cache. So existing import won't get the cached results, instead, evaluate with updated content.

(Adding suffixes like #random-number doesn't work, because of recursive imports)

CyanChanges avatar Jun 11 '25 03:06 CyanChanges

I don't think this makes much sense for a module loader api. It doesn't need any ability to mutate existing entries in the map, only to intercept new requests. This can be done without any new methods on the moduleMap itself, you only need the ModuleLoader trait.

I have updated the issue, added the the reasons and alternatives that other people and I have considered. I hope that would make sense to you 🙏

CyanChanges avatar Jun 11 '25 09:06 CyanChanges