fix: improved support for cjs and cts modules #26558

dsherret · 2024-10-25T23:27:50Z

cts support
better cjs/cts type checking
deno compile cjs/cts support
More efficient detect cjs (going towards stabilization)
- Determination of whether .js, .ts, .jsx, or .tsx is cjs or esm is only done after loading
feat: analyze commonjs files deno_graph#540
feat: use Program instead of Module more deno_ast#281
feat: analyze require calls as dynamic dependencies deno_ast#280
fix: ability to specify if transpiling cjs deno_ast#284

Closes #26651
Closes #25498

…_string

…to refactor_handle_program_more

…gram_more

Extracted out of #26558 Closes #26578

dsherret · 2024-10-30T19:46:01Z

cli/cache/module_info.rs

 pub struct ModuleInfoCache {
  conn: CacheDB,
+  parsed_source_cache: Arc<ParsedSourceCache>,


Shifted from being pased along horizontally to being a ctor dependency.

dsherret · 2024-10-30T19:49:21Z

cli/cache/module_info.rs

+    }
+  }
+
+  pub fn analyze_sync(


This sync version is for the require loader.

dsherret · 2024-10-30T19:50:07Z

cli/cache/parsed_source.rs

-    // this will conditionally parse because it's using a CapturingModuleParser
-    parser.parse_module(ParseOptions {
+    // this will conditionally parse because it's using a CapturingEsParser
+    parser.parse_program(ParseOptions {


Part of this PR is shifting everything over to using parse_program so that we can quickly tell if a module is definitely an ES module.

dsherret · 2024-10-30T20:12:23Z

cli/cache/parsed_source.rs

@@ -150,42 +147,3 @@ impl deno_graph::ParsedSourceStore for ParsedSourceCache {
    }
  }
 }
-
-pub struct EsmOrCjsChecker {


This was removed and replaced with CjsTracker.

dsherret · 2024-10-30T20:22:54Z

cli/lsp/documents.rs

 ) -> MediaType {
-  if resolver.in_node_modules(specifier) {


We don't change the media type from stuff like JS to TS when providing it to typescript anymore because this isn't something we can determine at resolution (only when loading). This is fine because we tell typescript about the "node module kind" (if ESM or CJS)

dsherret · 2024-10-30T20:27:31Z

cli/npm/byonm.rs

-      Ok(Cow::Borrowed(path))
-    }
-  }
-}


This was moved into the NodeRequireLoader

dsherret · 2024-10-30T20:28:39Z

cli/resolver.rs


-  pub fn url_to_node_resolution(


url_to_node_resolution is gone since we no longer determine if something is esm or cjs at resolution time, but rather after loading when we can see if it has imports/exports/tla/etc.

dsherret · 2024-10-30T20:36:30Z

resolvers/node/package_json.rs

+    Self { env }
+  }
+
+  pub fn get_closest_package_json(


All this code was extracted out of other places.

dsherret · 2024-10-30T20:36:50Z

resolvers/node/resolution.rs

@@ -73,16 +66,14 @@ impl NodeResolutionMode {

 #[derive(Debug)]
 pub enum NodeResolution {
-  Esm(Url),
-  CommonJs(Url),
+  Module(Url),


We no longer know this at resolution time.

dsherret · 2024-10-30T21:34:49Z

Cargo.toml

I'm sorry, this PR is hard to split up into multiple smaller ones. There's so much stuff that's intertwined.

The main themes are using "program" more and determining if a js, jsx, ts, or tsx file is cjs or esm always only happens after loading it now.

dsherret · 2024-10-31T00:05:33Z

ext/node/lib.rs

cc @arnauorriols on these changes and this PR in general. There's now a NodeRequireLoader trait for loading files with require(...) and PackageJsonResolver struct that searches for package.jsons. Let me know if you'd like a walkthrough of the changes.

…erret/deno into refactor_handle_program_more

bartlomieju

First pass, ended at cli/resolver.rs because the rest of files doesn't load in the browser :(

cli/args/mod.rs

cli/cache/module_info.rs

bartlomieju · 2024-11-01T02:54:00Z

cli/lsp/resolver.rs

+      // todo(dsherret): support in the lsp by stabilizing the feature
+      // so that we don't have to pipe the config in here


Maybe link it in an issue so it's not forgotten?

Or the PR that stabilizes it

The tracking issue has it: #26225 (just updated it to be more clear)

cli/main.rs

cli/module_loader.rs

bartlomieju · 2024-11-01T14:15:15Z

cli/tools/compile.rs

@@ -53,16 +53,6 @@ pub async fn compile(
    );
  }

-  if cli_options.unstable_detect_cjs() {


Is this on purpose? Shouldn't we land #26439 first?

Yes. It works in deno compile now so I got rid of the warning saying it doesn't work.

bartlomieju · 2024-11-01T14:19:46Z

cli/tsc/mod.rs

+      })
+      .unwrap_or(false);
+    // todo(dsherret): how to avoid cloning here?
+    Ok(code.to_string())


We can make OpLoadResponse accept Arc<str> and then the op_load would return a v8::Array that would manually serialize to v8::String

bartlomieju · 2024-11-01T14:22:28Z

ext/node/lib.rs

-
-  let source_code = format!(
-    r#"(function loadCjsModule(moduleName, isMain, inspectBrk) {{
-      Deno[Deno.internal].node.loadCjsModule(moduleName, isMain, inspectBrk);


loadCjsModule can probably be removed now. I can do that in a follow up

* cts support * better cjs/cts type checking * deno compile cjs/cts support * More efficient detect cjs (going towards stabilization) * Determination of whether .js, .ts, .jsx, or .tsx is cjs or esm is only done after loading * Support `import x = require(...);` Co-authored-by: Bartek Iwańczuk <[email protected]>

dsherret added 14 commits October 21, 2024 20:10

refactor: prefer parsing as Program over Module

c11079d

more updates

904c316

Starting on this, but going to revert.

4335aa0

Merge branch 'main' into refactor_handle_program_more

29876a5

compiling

7410544

working

34b1a88

Start on translating code

873da6b

perf: pass transpiled modules to deno_core as a string

342c81c

update

3cd7c94

Merge branch 'main' into perf_pass_transpiled_modules_to_deno_core_as…

eedcdca

…_string

Merge branch 'main' into refactor_handle_program_more

d1d7b36

update

8540344

Merge branch 'perf_pass_transpiled_modules_to_deno_core_as_string' in…

3a83933

…to refactor_handle_program_more

better cts support

af06589

dsherret changed the title ~~fix: cts support~~ fix: improved support for cjs and cts modules Oct 25, 2024

dsherret mentioned this pull request Oct 25, 2024

.cjs extension is supported, but not .cts in version 2 (rc) #25498

Closed

dsherret added 2 commits October 26, 2024 13:53

Merge branch 'main' into refactor_handle_program_more

21b5d06

fix(check): expose more globals from @types/node

eb9f9ff

dsherret mentioned this pull request Oct 28, 2024

fix(check): expose more globals from @types/node #26603

Merged

dsherret added 4 commits October 28, 2024 10:45

Merge branch 'fix_expose_types_node_globals' into refactor_handle_pro…

eaa4ea7

…gram_more

Merge branch 'main' into refactor_handle_program_more

941eefd

support in deno compile

9e7ad6f

working test

39e928f

dsherret added a commit that referenced this pull request Oct 28, 2024

fix(check): expose more globals from @types/node (#26603)

0e64163

Extracted out of #26558 Closes #26578

dsherret added 6 commits October 28, 2024 20:35

updates to pass around the module kind

bd824a6

Create PackageJsonResolver

01c773f

Use fs more

63acd1a

better cjs/esm tracking

84f1f70

compiling large refactor

ebaa01f

fix bug

cf9fd71

Merge branch 'main' into refactor_handle_program_more

ecde7bd

dsherret marked this pull request as ready for review October 30, 2024 21:33

dsherret commented Oct 30, 2024

View reviewed changes

dsherret requested review from bartlomieju and nathanwhit October 30, 2024 21:35

Bump ci

adcf6d3

dsherret commented Oct 31, 2024

View reviewed changes

dsherret mentioned this pull request Oct 31, 2024

Ability to require a file in the module graph without permissions #26644

Open

bartlomieju and others added 8 commits October 31, 2024 15:43

mark main CJS module

2636535

add a test for main cjs module

d47411b

Merge branch 'main' into refactor_handle_program_more

0693427

Update

66c139d

Support TsImportEquals

b2500dd

Merge branch 'refactor_handle_program_more' of https://github.com/dsh…

5e86e00

…erret/deno into refactor_handle_program_more

Merge branch 'main' into refactor_handle_program_more

ef85fc3

proper support for ts import equals and export equals

c21c283

dsherret mentioned this pull request Oct 31, 2024

feat: Wasm module support #26668

Merged

Add some more import equals tests

0411a73

Hajime-san mentioned this pull request Nov 1, 2024

Show better suggestions when CJS module resolution fails #26652

Open

bartlomieju reviewed Nov 1, 2024

View reviewed changes

This was referenced Nov 1, 2024

Source maps don't work with maybe cjs files #26679

Open

feat(node): stabilize detecting if CJS via "type": "commonjs" in a package.json #26439

Merged

bartlomieju approved these changes Nov 1, 2024

View reviewed changes

Merge branch 'main' into refactor_handle_program_more

a471da0

dsherret merged commit 826e42a into denoland:main Nov 1, 2024
17 checks passed

dsherret deleted the refactor_handle_program_more branch November 1, 2024 16:27

nathanwhit mentioned this pull request Nov 2, 2024

Relative path in import map appears to break package.json dependency resolution #26624

Closed

nathanwhit mentioned this pull request Nov 6, 2024

Fails to execute install script with ERR_MODULE_NOT_FOUND #26742

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: improved support for cjs and cts modules #26558

fix: improved support for cjs and cts modules #26558

dsherret commented Oct 25, 2024 •

edited

Loading

dsherret Oct 30, 2024

dsherret Oct 30, 2024

dsherret Oct 30, 2024

dsherret Oct 30, 2024

dsherret Oct 30, 2024

dsherret Oct 30, 2024

dsherret Oct 30, 2024

dsherret Oct 30, 2024

dsherret Oct 30, 2024

dsherret Oct 30, 2024

dsherret Oct 31, 2024

bartlomieju left a comment

bartlomieju Nov 1, 2024

bartlomieju Nov 1, 2024

dsherret Nov 1, 2024

bartlomieju Nov 1, 2024

dsherret Nov 1, 2024

bartlomieju Nov 1, 2024

bartlomieju Nov 1, 2024

		// todo(dsherret): support in the lsp by stabilizing the feature
		// so that we don't have to pipe the config in here

fix: improved support for cjs and cts modules #26558

fix: improved support for cjs and cts modules #26558

Conversation

dsherret commented Oct 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bartlomieju left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsherret commented Oct 25, 2024 •

edited

Loading