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

feat: lazy loaded esm sources #263

Merged
merged 32 commits into from Dec 7, 2023
Merged

feat: lazy loaded esm sources #263

merged 32 commits into from Dec 7, 2023

Conversation

crowlKats
Copy link
Member

@crowlKats crowlKats commented Oct 11, 2023

This commit adds a new lazy to lazy load internal runtime code
authored in ES modules using "op_lazy_load_esm" op.

The code needs to embedded in the binary to be eliglble for lazy-loading,
and that can be achieved using "lazy_loaded_esm" option on the
"extension!" macro.

Prerequisite for denoland/deno#20812

@CLAassistant
Copy link

CLAassistant commented Oct 11, 2023

CLA assistant check
All committers have signed the CLA.

core/ops_builtin_v8.rs Outdated Show resolved Hide resolved
core/ops_builtin_v8.rs Outdated Show resolved Hide resolved
core/ops_builtin_v8.rs Outdated Show resolved Hide resolved
core/ops_builtin_v8.rs Outdated Show resolved Hide resolved
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by self-review...

core/00_primordials.js Outdated Show resolved Hide resolved
core/01_core.js Outdated Show resolved Hide resolved
core/modules/loaders.rs Show resolved Hide resolved
core/modules/loaders.rs Outdated Show resolved Hide resolved
core/modules/map.rs Outdated Show resolved Hide resolved
core/modules/module_map_data.rs Outdated Show resolved Hide resolved
core/modules/testdata/lazy_loaded.js Show resolved Hide resolved
core/modules/tests.rs Outdated Show resolved Hide resolved
core/ops_builtin_v8.rs Outdated Show resolved Hide resolved
core/runtime/jsruntime.rs Outdated Show resolved Hide resolved
@bartlomieju
Copy link
Member

bartlomieju commented Dec 4, 2023

Pretty much stuck on how to handle primordials now...

EDIT: Spitballing here, but maybe we could add ext:deno_core/primordials.js that essentially reexports what's available in globalThis.__bootstrap.primordials. It still wouldn't be exposed to the user code, but would be available to any extension to access it.

EDIT2: After some more thinking, I think ext:deno_core/mod.js is the way to go - it would export core, internals and primordials. That way embedders that author code as modules can still use them, but embedders using ES modules can import them instead of capturing globalThis.__bootstrap.

@bartlomieju
Copy link
Member

Opened #363 to solve the primordials problem.

bartlomieju added a commit that referenced this pull request Dec 5, 2023
This commit adds "ext:core/mod.js" built-in ES module that reexports
"core", "internals" and "primordials" properties of the 
"globalThis.__bootstrap" namespace. This is very convenient for
embedders that author runtime code using ES modules instead of
scripts, because it allows to import these props directly instead of
capturing "globalThis.__bootstrap" namespace.

To achieve that a new "ModuleMap::lazy_load_es_module_from_code"
method was added that accepts a specifier and source code; instantiates
and evaluates the provided code as ES module.

This will be very useful for
#263
and denoland/deno#21422.
@bartlomieju bartlomieju marked this pull request as ready for review December 6, 2023 23:10
@bartlomieju bartlomieju changed the title feat: lazy load esm feat: lazy loaded esm sources Dec 6, 2023
core/modules/loaders.rs Outdated Show resolved Hide resolved
"setup.js",
r#"
Deno.core.print("1\n");
const module = Deno.core.ops.op_lazy_load_esm("ext:test_ext/lazy_loaded.js");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add this as built-in on core? Maybe Deno.core.importSync

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not - I think it's good that it's in ops and not too obvious. User code can still execute it (but the blast radius is really limited), and I think ops a bit more obscure than APIs on Deno.core.

@mmastrac
Copy link
Contributor

mmastrac commented Dec 6, 2023

This looks good to me other than some minor items.

@bartlomieju bartlomieju merged commit 62087f5 into main Dec 7, 2023
5 checks passed
@bartlomieju bartlomieju deleted the lazy-load-esm branch December 7, 2023 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants