nodejs
deps: update temporal to 0.1.2
Why not? It's a wip feature, behind a flag and our minimal tests don't fail.
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 https://github.com/v8/v8/commit/0cbecc1aa10df9ce1e5380a76fc5711f565491d5), 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
Maybe we should reconsider having a script to auto-update this dependency if it's tightly coupled with V8
Or said another way: change the V8 update script so it also updates temporal with right version.
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
~~But surprisingly, the CI on this PR is green?~~ definitely, we don't have any temporal CI.
Maybe we should reconsider having a script to auto-update this dependency if it's tightly coupled with V8
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 deps/temporal via --v8-enable-temporal-support pulls down a bunch of dependencies via cargo from the Internet). The official way is cargo vendor (and then at build time passing --offline to cargo).
If I run that in deps/temporal, I end up with a vendor subdirectory that is 119MB in size... which is not too far off Google's own vendored rust crates in https://chromium.googlesource.com/chromium/src/third_party/rust/+/4d93511ebaceb09ebdd83c8876a4a936b75fa04d (from https://github.com/nodejs/node/blob/746c3c2ec2eee4f33adde73c34364ed873cacffd/deps/v8/DEPS#L508-L509) which is 157MB.
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 third_party/rust dependency and have that update (via @node-core/utils) when V8 is.
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 main so the possibility is real.
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.
chromium/src/third_party/rust/ does not include every crate that cargo vendor would do. Like windows_aarch64_gnullvm, windows_i686_gnullvm, windows_x86_64_gnu etc, which consist a large portion of the size cargo vendor creates. We should probably do the similar trimming on the vendored sources.
Refs: https://chromium.googlesource.com/chromium/src/+/refs/heads/main/tools/crates/gnrt/removed_crate.md
@legendecas Yeah, the trimming that Chromium already do is another point in favour of using chromium/src/third_party/rust/ instead of trying to replicate that (and any patching) ourselves.
Planning for when eventually temporal is enabled by default, we'll need to vendor (either ourselves or reusing Chromium's chromium/src/third_party/rust/). One question is whether we want to be able to update temporal_capi independently of V8? We do for, e.g. zlib, icu, but do not for e.g. simdutf.
The issue with chromium/src/third_party/rust/ is that it is not dedicated for V8 only dependencies. It also includes dependencies used by Chromium and not going to be used by Node.js (in the foreseeable future), like png, qr_code, font libraries read-fonts and skrifa (link).
We can not stop chromium from adding more dependencies we don't need here in Node.js to chromium/src/third_party/rust/.
We can enumerate the dependencies that are needed for temporal_capi and gitignore the other ones.
We can enumerate the dependencies that are needed for temporal_capi and gitignore the other ones.
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 -e normal
[dependencies.temporal_capi]
version = "0.1.2"
features = ["zoneinfo64"]
[dependencies.temporal_rs]
version = "0.1.2"
default-features = false
# This is necessary to enable a spec-compliance quirk
features = ["float64_representable_durations"]
cargo tree output
$ cargo tree -e normal
foo v0.1.0 (/home/manishearth/sand/crates/foo)
├── temporal_capi v0.1.2
│ ├── diplomat v0.14.0 (proc-macro)
│ │ ├── diplomat_core v0.14.0
│ │ │ ├── proc-macro2 v1.0.103
│ │ │ │ └── unicode-ident v1.0.20
│ │ │ ├── quote v1.0.41
│ │ │ │ └── proc-macro2 v1.0.103 (*)
│ │ │ ├── serde v1.0.228
│ │ │ │ ├── serde_core v1.0.228
│ │ │ │ └── serde_derive v1.0.228 (proc-macro)
│ │ │ │ ├── proc-macro2 v1.0.103 (*)
│ │ │ │ ├── quote v1.0.41 (*)
│ │ │ │ └── syn v2.0.108
│ │ │ │ ├── proc-macro2 v1.0.103 (*)
│ │ │ │ ├── quote v1.0.41 (*)
│ │ │ │ └── unicode-ident v1.0.20
│ │ │ ├── smallvec v1.15.1
│ │ │ ├── strck v1.0.0
│ │ │ │ └── unicode-ident v1.0.20
│ │ │ └── syn v2.0.108 (*)
│ │ ├── proc-macro2 v1.0.103 (*)
│ │ ├── quote v1.0.41 (*)
│ │ └── syn v2.0.108 (*)
│ ├── diplomat-runtime v0.14.0
│ ├── icu_calendar v2.1.0
│ │ ├── calendrical_calculations v0.2.3
│ │ │ ├── core_maths v0.1.1
│ │ │ │ └── libm v0.2.15
│ │ │ └── displaydoc v0.2.5 (proc-macro)
│ │ │ ├── proc-macro2 v1.0.103 (*)
│ │ │ ├── quote v1.0.41 (*)
│ │ │ └── syn v2.0.108 (*)
│ │ ├── displaydoc v0.2.5 (proc-macro) (*)
│ │ ├── icu_calendar_data v2.1.1
│ │ ├── icu_locale v2.1.1
│ │ │ ├── icu_collections v2.1.1
│ │ │ │ ├── displaydoc v0.2.5 (proc-macro) (*)
│ │ │ │ ├── potential_utf v0.1.4
│ │ │ │ │ ├── writeable v0.6.2
│ │ │ │ │ └── zerovec v0.11.5
│ │ │ │ │ ├── yoke v0.8.1
│ │ │ │ │ │ ├── stable_deref_trait v1.2.1
│ │ │ │ │ │ ├── yoke-derive v0.8.1 (proc-macro)
│ │ │ │ │ │ │ ├── proc-macro2 v1.0.103 (*)
│ │ │ │ │ │ │ ├── quote v1.0.41 (*)
│ │ │ │ │ │ │ ├── syn v2.0.108 (*)
│ │ │ │ │ │ │ └── synstructure v0.13.2
│ │ │ │ │ │ │ ├── proc-macro2 v1.0.103 (*)
│ │ │ │ │ │ │ ├── quote v1.0.41 (*)
│ │ │ │ │ │ │ └── syn v2.0.108 (*)
│ │ │ │ │ │ └── zerofrom v0.1.6
│ │ │ │ │ │ └── zerofrom-derive v0.1.6 (proc-macro)
│ │ │ │ │ │ ├── proc-macro2 v1.0.103 (*)
│ │ │ │ │ │ ├── quote v1.0.41 (*)
│ │ │ │ │ │ ├── syn v2.0.108 (*)
│ │ │ │ │ │ └── synstructure v0.13.2 (*)
│ │ │ │ │ ├── zerofrom v0.1.6 (*)
│ │ │ │ │ └── zerovec-derive v0.11.2 (proc-macro)
│ │ │ │ │ ├── proc-macro2 v1.0.103 (*)
│ │ │ │ │ ├── quote v1.0.41 (*)
│ │ │ │ │ └── syn v2.0.108 (*)
│ │ │ │ ├── yoke v0.8.1 (*)
│ │ │ │ ├── zerofrom v0.1.6 (*)
│ │ │ │ └── zerovec v0.11.5 (*)
│ │ │ ├── icu_locale_core v2.1.1
│ │ │ │ ├── displaydoc v0.2.5 (proc-macro) (*)
│ │ │ │ ├── litemap v0.8.1
│ │ │ │ ├── tinystr v0.8.2
│ │ │ │ │ ├── displaydoc v0.2.5 (proc-macro) (*)
│ │ │ │ │ └── zerovec v0.11.5 (*)
│ │ │ │ ├── writeable v0.6.2
│ │ │ │ └── zerovec v0.11.5 (*)
│ │ │ ├── icu_locale_data v2.1.1
│ │ │ ├── icu_provider v2.1.1
│ │ │ │ ├── displaydoc v0.2.5 (proc-macro) (*)
│ │ │ │ ├── icu_locale_core v2.1.1 (*)
│ │ │ │ ├── stable_deref_trait v1.2.1
│ │ │ │ ├── writeable v0.6.2
│ │ │ │ ├── yoke v0.8.1 (*)
│ │ │ │ ├── zerofrom v0.1.6 (*)
│ │ │ │ ├── zerotrie v0.2.3
│ │ │ │ │ └── displaydoc v0.2.5 (proc-macro) (*)
│ │ │ │ └── zerovec v0.11.5 (*)
│ │ │ ├── potential_utf v0.1.4 (*)
│ │ │ ├── tinystr v0.8.2 (*)
│ │ │ └── zerovec v0.11.5 (*)
│ │ ├── icu_locale_core v2.1.1 (*)
│ │ ├── icu_provider v2.1.1 (*)
│ │ ├── tinystr v0.8.2 (*)
│ │ └── zerovec v0.11.5 (*)
│ ├── icu_locale v2.1.1 (*)
│ ├── num-traits v0.2.19
│ ├── temporal_rs v0.1.2
│ │ ├── core_maths v0.1.1 (*)
│ │ ├── icu_calendar v2.1.0 (*)
│ │ ├── icu_locale v2.1.1 (*)
│ │ ├── ixdtf v0.6.4
│ │ ├── num-traits v0.2.19
│ │ ├── timezone_provider v0.1.2
│ │ │ ├── tinystr v0.8.2 (*)
│ │ │ ├── zerotrie v0.2.3 (*)
│ │ │ ├── zerovec v0.11.5 (*)
│ │ │ └── zoneinfo64 v0.2.1
│ │ │ ├── calendrical_calculations v0.2.3 (*)
│ │ │ ├── icu_locale_core v2.1.1 (*)
│ │ │ ├── potential_utf v0.1.4 (*)
│ │ │ ├── resb v0.1.1
│ │ │ │ ├── potential_utf v0.1.4 (*)
│ │ │ │ └── serde_core v1.0.228
│ │ │ └── serde v1.0.228
│ │ │ ├── serde_core v1.0.228
│ │ │ └── serde_derive v1.0.228 (proc-macro) (*)
│ │ ├── tinystr v0.8.2 (*)
│ │ └── writeable v0.6.2
│ ├── timezone_provider v0.1.2 (*)
│ ├── writeable v0.6.2
│ └── zoneinfo64 v0.2.1 (*)
└── temporal_rs v0.1.2 (*)
Currently, 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.
