Skip to content

match deno@2.3.3 dependencies#367

Merged
rscarson merged 2 commits into
rscarson:masterfrom
wbk:master
Aug 15, 2025
Merged

match deno@2.3.3 dependencies#367
rscarson merged 2 commits into
rscarson:masterfrom
wbk:master

Conversation

@wbk

@wbk wbk commented May 17, 2025

Copy link
Copy Markdown
Contributor

The libffi dependency introduced (by way of deno_ffi) by #366 has caused master to break for me on macos; I decided to take a stab at a refactor to support 2.3.3 instead.

@wbk

wbk commented May 20, 2025

Copy link
Copy Markdown
Contributor Author

I realize there are a lot of moving parts here, but there's so much codependence between all of deno's deps, I'm not certain I could have "done less". Are PRs introducing this amount of change welcome (and is there any better way I could structure this request?)

@rscarson

Copy link
Copy Markdown
Owner

Sorry! I have a newborn at home so I haven't had time to take a look yet!

I've approved the checks

Comment thread src/ext/mod.rs
ext.esm_files = ::std::borrow::Cow::Borrowed(&[]);
ext.esm_entry_point = ::std::option::Option::None;
}
// Clears the js and esm files for warmup to avoid reloading them

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm skeptical this is still necessary post-denoland/deno_core@aa6f360 (In fact, I think this entire trait can probably go away in favor of calling Extension::init directly)

let referrer_specifier = referrer
.to_module_specifier(&self.cwd)
.map_err(|e| JsErrorBox::from_err(to_io_err(e)))?;
let referrer_specifier = if deno_core::specifier_has_uri_scheme(referrer) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was needed to fix one of the module loader unit tests, deno seems to be storing module paths internally as file:/// urls rather than preserving the relative paths from source

Comment thread src/ext/node/resolvers.rs
use super::cjs_translator::{NodeCodeTranslator, RustyCjsCodeAnalyzer};

const NODE_MODULES_DIR: &str = "node_modules";
const TYPESCRIPT_VERSION: &str = "5.8.3";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@wbk

wbk commented May 20, 2025

Copy link
Copy Markdown
Contributor Author

No worries and congrats! I left a few callouts as notes

@getong

getong commented Aug 15, 2025

Copy link
Copy Markdown

this pr is great, would somebody take a look at it and merge it?

@rscarson rscarson merged commit 1821240 into rscarson:master Aug 15, 2025
5 checks passed
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.

3 participants