Uh oh!
There was an error while loading. Please reload this page.
deps: update temporal to 0.1.2 - #60897
Conversation
nodejs-github-bot
commented
Nov 30, 2025
Review requested:
|
aduh95
left a comment
There was a problem hiding this comment.
IIUC we don't want to update before we update V8 to 14.4
targos
commented
Dec 1, 2025
Why not? It's a wip feature, behind a flag and our minimal tests don't fail. |
aduh95
commented
Dec 1, 2025
The two versions are not compatible AFAIK, we would need to either cherry-pick the commits that landed on V8 to make it compatible (at least v8/v8@0cbecc1), accept a broken state, or simply not land this until we update the version of V8. The former seems hardly worth it (as you said we don't use it officially anyway), so the latter seems the best course of action as I don't understand what would be the upside of purposefully land something that we know to be broken |
targos
commented
Dec 1, 2025
Maybe we should reconsider having a script to auto-update this dependency if it's tightly coupled with V8 |
targos
commented
Dec 1, 2025
Or said another way: change the V8 update script so it also updates temporal with right version. |
aduh95
commented
Dec 1, 2025
That would be ideal yes, unfortunately I found it harder than I anticipated to set up. I think V8 14.4 should be buildable with that version, but yeah it could become a maintenance burden |
|
richardlau
commented
Dec 3, 2025
I had some time today to look at temporal and what we might need to do to vendor everything into the Node.js source tree (currently building If I run that in Lines 508 to 509 in 746c3c2 Point here being that if we think the version of temporal is going to be coupled to the V8 version, the easiest solution (at the cost of disk space) would be to vendor in V8's Another advantage to using Google's vendored crates is that we would pick up patches from https://chromium.googlesource.com/chromium/src/third_party/rust/+/4d93511ebaceb09ebdd83c8876a4a936b75fa04d/chromium_crates_io/patches/. There isn't one for temporal at the commit for the version of V8 we're currently using, but there is one on their |
aduh95
commented
Dec 3, 2025
157 MiB sounds like a lot! Looking at https://github.com/nodejs/node/actions/runs/19791503862?pr=60897, currently the source code + vendored deps is about 107MiB gzipped (the source code without sharable deps is about 48 MiB gzipped, according to https://github.com/nodejs/node/actions/runs/19903600039). If that's the solution we go with, it wouldn't make sense to backport it to v25.x. |
legendecas
commented
Dec 4, 2025
Refs: https://chromium.googlesource.com/chromium/src/+/refs/heads/main/tools/crates/gnrt/removed_crate.md |
richardlau
commented
Dec 4, 2025
@legendecas Yeah, the trimming that Chromium already do is another point in favour of using Planning for when eventually temporal is enabled by default, we'll need to vendor (either ourselves or reusing Chromium's |
legendecas
commented
Dec 4, 2025
The issue with We can not stop chromium from adding more dependencies we don't need here in Node.js to |
targos
commented
Dec 4, 2025
We can enumerate the dependencies that are needed for |
Manishearth
commented
Dec 4, 2025
Yeah, this is what I would recommend. When temporal_rs updates there is a chance that more dependencies will be needed, but that can be pretty easily noticed by CI. The way to get this list easily is to take the Temporal-relevant dependencies from Chrome's Cargo.toml, stick them in a crate, and run cargo tree outputCurrently, that boils down to this list of crates: calendrical_calculations, core_maths, diplomat, diplomat_core, diplomat-runtime, displaydoc, icu_calendar, icu_calendar_data, icu_collections, icu_locale, icu_locale_core, icu_locale_data, icu_provider, ixdtf, libm, litemap, num-traits, potential_utf, proc-macro2, quote, resb, serde, serde_core, serde_derive, smallvec, stable_deref_trait, strck, syn, synstructure, temporal_capi, temporal_rs, timezone_provider, tinystr, unicode-ident, writeable, yoke, yoke-derive, zerofrom, zerofrom-derive, zerotrie, zerovec, zerovec-derive, zoneinfo64. |
5af743c to
cee75ccComparecee75cc to
6bf7b8fCompare6bf7b8f to
acb9b13Comparelegendecas
commented
Dec 28, 2025
Closing as the crate deps have been changed in #61072. |
This is an automated update of temporal to 0.1.2.