feat(unstable): initial support for npm specifiers #15484

dsherret · 2022-08-16T22:21:32Z

This adds initial very rough support for npm specifiers for deno run, deno eval, and deno test.

Packages and their registry info are downloaded & cached to the deno dir. Reloading registry info can be done by using the --reload flag.
Node globals are limited to the npm packages. For example, this allows for the node code to use a different setTimeout implementation.
No type checking, lsp, deno run npm:mkdirp, deno info, deno vendor, or lockfile support yet. These will land in separate PRs.

Maybes before landing:

Analyzing the "native" node modules with deno graph is incredibly slow. The need to do this should be eliminated somehow.

Co-authored-by: Bartek Iwańczuk [email protected]

… into npm_deno_run_support

…quire_resolve_exports

ry · 2022-08-18T21:08:00Z

Excluding the testdata

# git diff main --stat -- ':!cli/tests/testdata/npm/registry/'
 Cargo.lock                                        |   4 +
 cli/Cargo.toml                                    |   1 +
 cli/cache/mod.rs                                  |  17 ++++
 cli/compat/esm_resolver.rs                        |   3 +-
 cli/compat/mod.rs                                 |  41 +++------
 cli/graph_util.rs                                 |  28 +++++-
 cli/lsp/cache.rs                                  |   4 +-
 cli/lsp/diagnostics.rs                            |   3 +
 cli/main.rs                                       |   2 +-
 cli/module_loader.rs                              | 103 ++++++++++++++--------
 cli/node/analyze.rs                               | 144 ++++++++++++++++++++++++++++++
 cli/node/mod.rs                                   | 697 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 cli/npm/cache.rs                                  |  43 +++++----
 cli/npm/mod.rs                                    | 171 ++++++++++++++++++++++++++++++------
 cli/npm/registry.rs                               |  35 +++++++-
 cli/npm/resolution.rs                             |  46 +++++++---
 cli/proc_state.rs                                 | 117 +++++++++++++++++++++----
 cli/standalone.rs                                 |   1 +
 cli/tests/integration/mod.rs                      |   2 +
 cli/tests/integration/npm_tests.rs                | 123 ++++++++++++++++++++++++++
 cli/tests/testdata/commonjs/init.js               |  11 +--
 cli/tests/testdata/npm/cjs_sub_path/main.js       |  21 +++++
 cli/tests/testdata/npm/cjs_sub_path/main.out      |  16 ++++
 cli/tests/testdata/npm/cjs_with_deps/main.js      |  12 +++
 cli/tests/testdata/npm/cjs_with_deps/main.out     |  15 ++++
 cli/tests/testdata/npm/dynamic_import/main.out    |   4 +
 cli/tests/testdata/npm/dynamic_import/main.ts     |   3 +
 cli/tests/testdata/npm/dynamic_import/other.ts    |   4 +
 cli/tests/testdata/npm/esm/main.js                |   7 ++
 cli/tests/testdata/npm/esm/main.out               |   2 +
 cli/tests/testdata/npm/esm/test.js                |   5 ++
 cli/tests/testdata/npm/esm/test.out               |  12 +++
 cli/tests/testdata/npm/import_map/import_map.json |   5 ++
 cli/tests/testdata/npm/import_map/main.js         |   7 ++
 cli/tests/testdata/npm/import_map/main.out        |   2 +
 cli/worker.rs                                     | 114 ++++++++++++++++--------
 ext/node/01_node.js                               | 111 ++++++++++++++++++++++++
 ext/node/{01_require.js => 02_require.js}         | 212 +++++++++++++++++++++++++++++----------------
 ext/node/Cargo.toml                               |   2 +
 ext/node/errors.rs                                | 122 ++++++++++++++++++++++++++
 ext/node/lib.rs                                   | 380 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------
 ext/node/package_json.rs                          | 158 +++++++++++++++++++++++++++++++++
 ext/node/resolution.rs                            | 696 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 runtime/build.rs                                  |   2 +-
 runtime/examples/hello_runtime.rs                 |   1 +
 runtime/web_worker.rs                             |   4 +-
 runtime/worker.rs                                 |   5 +-
 test_util/Cargo.toml                              |   1 +
 test_util/src/lib.rs                              |  72 ++++++++++++++-
 49 files changed, 3238 insertions(+), 353 deletions(-)

ry

Great work @dsherret !

I'm concerned about the deno_graph slowness, it's going to be pretty apparent to users.

cli/compat/mod.rs

cli/npm/resolution.rs

ry · 2022-08-19T00:33:49Z

cli/proc_state.rs

+
+      // add the builtin node modules to the graph data
+      let node_std_graph = self
+        .create_graph(vec![(compat::MODULE_ALL_URL.clone(), ModuleKind::Esm)])


The slowness is quite bad... it takes almost 2 seconds to load a very basic npm dependency that's already been downloaded.

cli/tests/integration/npm_tests.rs

cli/node/analyze.rs

ry · 2022-08-19T11:37:39Z

cli/node/mod.rs

+) -> Result<(), AnyError> {
+  let source_code = &format!(
+    r#"(async function loadBuiltinNodeModules(moduleAllUrl) {{
+      const moduleAll = await import(moduleAllUrl);


It's unfortunate that we have to load all of the node modules even if only some are used. Do you think in the future these could be loaded lazily?

Unlikely, these modules have a lot of cross-imports, so they would be loaded anyway

I'd push back on that assumption. I'm sure there are many npm modules that don't require, say, crypto.

ry · 2022-08-19T11:40:44Z

cli/node/mod.rs

+/// For all discovered reexports the analysis will be performed recursively.
+///
+/// If successful a source code for equivalent ES module is returned.
+pub fn translate_cjs_to_esm(


would be great to have a little unit test for this...

There are integration tests that cover this API

cli/node/mod.rs

ext/node/package_json.rs

ry

LGTM

dsherret and others added 30 commits August 11, 2022 11:31

Start of deno run support.

25797ac

refactor: consolidate most runtime code to the same place

84ea861

Merge branch 'main' into npm_deno_run_support

7e02847

Merge branch 'refactor_workers' into npm_deno_run_support

9c58104

More work.

62e0d49

Merge branch 'main' into npm_deno_run_support

9f644eb

Improvements.

f7c9abb

Permission check that doesn't panic.

61e0706

wip

aa5f706

resolveExports

fb8d0d4

resolveExports, 2

fb3c2a0

readPackageScope

c3d45a7

trySelf

c9d77d5

disable perm check, inject process global

cca72da

make it work with relative specifiers

3687c7e

reenable perm checks

8783063

Merge branch 'require_exports' into npm_deno_run_support

ae8c101

remove unneeded code

270aac9

Use small mammal resolution only for now

e91a34d

Fix resolving bare specifiers like <package>/<path>

db426e2

Merge branch 'main' into npm_deno_run_support

c0eabe2

handle conditional imports in require

64cf64b

Merge branch 'npm_deno_run_support' of https://github.com/denoland/deno…

e5a7176

… into npm_deno_run_support

Add back the ability for package.json to not exist.

22a2007

Ensure exported functions are bound to their module object

e469722

Fix comment.

c8dafa0

Reuse more code in ext/node in the cli

a6a27ec

Handle exports in cjs -> esm that are reserved words.

9a8fe85

Fix windows issue and ensure path is return instead of url from op_re…

81dc03f

…quire_resolve_exports

Primordial.

947918c

dsherret and others added 12 commits August 17, 2022 10:08

Fix parsing version requirements with leading 'v'

eb159fc

Add node global "global"

ce99e77

Add REQUIRE_CONDITIONS

6f202bc

Add node to require condition as well

161ce4b

Fix bugs and ability to update npm registry directory automatically.

98ddf27

Make env var shorter

4a3a796

Format code.

82c890f

add some permissions checks

9cc1724

Merge branch 'main' into npm_deno_run_support

d91d02a

Merge branch 'main' into npm_deno_run_support

b9aa7bf

refactor ensure_read_permission

e65f040

add missing copyright

4c36f91

ry reviewed Aug 19, 2022

View reviewed changes

Merge branch 'main' into npm_deno_run_support

ff0ad78

ry reviewed Aug 19, 2022

View reviewed changes

cli/node/analyze.rs Outdated Show resolved Hide resolved

cli/node/analyze.rs Show resolved Hide resolved

cli/node/analyze.rs Show resolved Hide resolved

ry reviewed Aug 19, 2022

View reviewed changes

cli/node/mod.rs Outdated Show resolved Hide resolved

ry reviewed Aug 19, 2022

View reviewed changes

cli/node/mod.rs Outdated Show resolved Hide resolved

ext/node/package_json.rs Show resolved Hide resolved

ext/node/package_json.rs Show resolved Hide resolved

bartlomieju added 5 commits August 19, 2022 14:36

Merge branch 'main' into npm_deno_run_support

3af0072

add test for name_without_path

1d3ed7f

test_esm_code_with_node_globals

0fe3752

add tests, address review feedback

e29d400

Merge branch 'main' into npm_deno_run_support

e01a771

ry approved these changes Aug 20, 2022

View reviewed changes

dsherret merged commit 87f80ff into main Aug 20, 2022

dsherret deleted the npm_deno_run_support branch August 20, 2022 15:31

dsherret mentioned this pull request Aug 22, 2022

Support npm URLs #13703

Closed

kdziamura mentioned this pull request Jan 13, 2023

fix(npm): use original node regex in npm resolution #17404

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(unstable): initial support for npm specifiers #15484

feat(unstable): initial support for npm specifiers #15484

dsherret commented Aug 16, 2022

ry commented Aug 18, 2022

ry left a comment

ry Aug 19, 2022

ry Aug 19, 2022

bartlomieju Aug 19, 2022

ry Aug 19, 2022

ry Aug 19, 2022

bartlomieju Aug 19, 2022

ry left a comment

feat(unstable): initial support for npm specifiers #15484

feat(unstable): initial support for npm specifiers #15484

Conversation

dsherret commented Aug 16, 2022

ry commented Aug 18, 2022

ry left a comment

Choose a reason for hiding this comment

ry Aug 19, 2022

Choose a reason for hiding this comment

ry Aug 19, 2022

Choose a reason for hiding this comment

bartlomieju Aug 19, 2022

Choose a reason for hiding this comment

ry Aug 19, 2022

Choose a reason for hiding this comment

ry Aug 19, 2022

Choose a reason for hiding this comment

bartlomieju Aug 19, 2022

Choose a reason for hiding this comment

ry left a comment

Choose a reason for hiding this comment