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: upgrade V8 to 12.8 #24693

Merged
merged 3 commits into from
Jul 31, 2024
Merged

feat: upgrade V8 to 12.8 #24693

merged 3 commits into from
Jul 31, 2024

Conversation

devsnek
Copy link
Member

@devsnek devsnek commented Jul 24, 2024

  • upgrade to v8 12.8
    • optimizes DataView bigint methods
    • fixes global interceptors
    • includes CPED methods for ALS
  • fix global resolution
    • makes global resolution consistent using host_defined_options. originally a separate patch but due to the global interceptor bug it needs to be included in this pr for all tests to pass.

@devsnek devsnek force-pushed the upgrade-v8 branch 6 times, most recently from 4db1498 to 6e6c160 Compare July 27, 2024 22:21
@bartlomieju bartlomieju added this to the 1.46 milestone Jul 28, 2024
ext/node_resolver/sync.rs Outdated Show resolved Hide resolved
@bartlomieju
Copy link
Member

Let's change the title to feat: upgrade V8 to 12.8 before we land this one

@devsnek devsnek changed the title feat: upgrade deno_core feat: upgrade V8 to 12.8 Jul 31, 2024
@devsnek devsnek requested a review from bartlomieju July 31, 2024 22:28
@bartlomieju
Copy link
Member

Please rebase

ext/node/lib.rs Outdated
@@ -836,3 +836,12 @@ impl<'a> deno_package_json::fs::DenoPkgJsonFs for DenoPkgJsonFsAdapter<'a> {
.map_err(|err| err.into_io_error())
}
}

pub fn get_host_defined_options<'s>(
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this one be named create_host_defined_options?

let source = v8::script_compiler::Source::new(source_str, None);
let resource_name = v8::undefined(scope);
let host_defined_options = get_host_defined_options(scope);
let origin = v8::ScriptOrigin::new(
Copy link
Member

Choose a reason for hiding this comment

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

Can we please a new API for v8::ScriptOrigin? 🙏 It should accept a scope and an options struct, with only resource_name being required.

const compiledWrapper = wrapSafe(filename, content, this, format);

if (format === "module") {
// TODO: implement require esm
Copy link
Member

Choose a reason for hiding this comment

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

Can you please open an issue and link it here?

if (StringPrototypeEndsWith(filename, ".js")) {
const pkg = op_require_read_closest_package_json(filename);
if (pkg && pkg.typ === "module") {
if (pkg?.typ === "module") {
// TODO: implement require esm
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

@devsnek devsnek enabled auto-merge (squash) July 31, 2024 22:45
@devsnek devsnek merged commit f57745f into main Jul 31, 2024
17 checks passed
@devsnek devsnek deleted the upgrade-v8 branch July 31, 2024 23:22
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