tc39 / proposal-compartments

Compartmentalization of host behavior hooks for JS
MIT License
122 stars 10 forks source link

To import(module) or to module.import() #85

Closed kriskowal closed 2 years ago

kriskowal commented 2 years ago

As written today, module blocks and the Module constructor produce a Module instance and to get a namespace for a module instance, we overload dynamic import.

import module m from 'example';
// or
const m = module {};
// or
const m = await import.module('example');

// and

await import(m);

I propose that we change this such that Module.prototype.import does the work.

await m.import();

And also add a load method that performs the transitive loading of the module’s dependencies.

await m.load();

That would place us in a coherent position, where future growth of module features would go on the Module.prototype and require no additional syntax or overloading.

However, the above does not account for all the ways a module want to express to tooling whether they need the module loaded, the module and its transitive dependencies loaded, or the module and its transitive dependencies initialized.

The module reflection proposal necessarily answers the question of how to import a Module object but I would need to look closer to know whether it also incidentally signals to bundlers that the transitive dependencies must be loaded and whether it’s possible to just capture the source of one module. For the purpose of a bundler, static syntax is not necessary. It would be sufficient if import.module(path) obtained a single module and m.load() brought in the transitive dependencies.

nicolo-ribaudo commented 2 years ago

Bundler hints sound something that would be covered by the possible "evaluator attributes" proposal, which different people talked about, especially tool authors, but no one wrote down yet. It's glorified syntax for query parameters, from an ECMA-262 point of view.

import module m from "example" with { preloadDependencies: true }; 

For the runtime usage, m.load() is polyfillable (I did it in this gist), but it's hard to do so having it as a built-in method would help.

I am overall positive about m.load(), but replacing import(m) with m.import() for symmetry with m.load() breaks the symmetry between importing a pre-loaded module and importing a new module (i.e. with a module instance and with a string):

devsnek commented 2 years ago

I think my immediate expectation is that import(m) would be the full import flow, and the module instance would have primitives, load, evaluate, getNamespace that you could combine, rather than an import method, especially since import() is available everywhere anyway.

Jack-Works commented 2 years ago

I prefer to have both import(x) and x.import()

kriskowal commented 2 years ago

From out-of-band conversations, starting with @mhofman, we’ve come to realize that all I/O, both import and load behavior, really needs to be funneled through dynamic import syntax. In my opinion, this is ugly, but without this limitation, the Module prototype becomes an irrevocable leak of I/O authority, and with module blocks, access to the Module.prototype becomes undeniable. This also means that the champions are also leaning toward the forms proposed by champions of import reflection, e.g., import(x, { reflect: true }) over my evidently misguided preference for import.module(x). You can expect updates from me.

I would like to withdraw this proposal entirely.

Jack-Works commented 2 years ago

the Module prototype becomes an irrevocable leak of I/O authority, and with module blocks, access to the Module.prototype becomes undeniable.

Is that really leaks the IO authority?

const mod = new ModuleSource(`
    const block = module { import 'x' }
    import(block)
`)
new Module(mod, { importHook }).import()

The import 'x' will be captured by the importHook right?

mhofman commented 2 years ago

The main issue is with module blocks syntax, not the various constructors.

const mod = module {
  export {currentTime} from ‘https://my.api.server.com/get-time.js’;
};
mod[‘import’]().then(({currentTime}) => console.log(currentTime));
devsnek commented 2 years ago

how is that different from a module file with the same source text, imported by path?

mhofman commented 2 years ago

Currently all module I/O is either through static import or dynamic import. Static import can only happen in module code, not eval. Dynamic import is deniable by simple regular expressions, which systems like the SES shim and Locker have. Denying module blocks would require full AST, and is a lot more intrusive.

nicolo-ribaudo commented 2 years ago

I'm nitpicking here, but you can disallow module blocks with /module(?:\s|\/\*.*?\*\/)*\{/ which I believe is very similar to what you are using for dynamic imports. Except that for module blocks it is easier, because module can only be followed by single-line block comments.

Anyway, I prefer import() so I'm not going to disagree with your conclusion :P

ljharb commented 2 years ago

It’s also a tooling issue; there’s an extremely large amount of tooling for require and import/import(), and having to change all of that to also recognize a new form of import is very costly.

kriskowal commented 2 years ago

@ljharb Can you clarify which form for reflective import (import but promise for Module instance) is least costly?

ljharb commented 2 years ago

@kriskowal im primarily talking about tooling that looks at the specifier to understand module graphs - so i don’t think the tooling argument really applies as much to the value you get from the import (assuming dynamic import still always produces a promise)

caridy commented 2 years ago

I would like to withdraw this proposal entirely.

I concur with your conclusion. Let's keep it simple, import() imports the various forms of modules.