Skip to content

Conversation

@targos
Copy link
Member

@targos targos commented Nov 12, 2025

It won't work until we build temporal_capi, but is a step forward.

Refs: #58730

It won't work until we build `temporal_capi`, but is a step forward.

Refs: nodejs#58730
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency. labels Nov 12, 2025
@targos targos added dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. labels Nov 12, 2025
@targos targos added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Nov 12, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 12, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@aduh95
Copy link
Contributor

aduh95 commented Nov 12, 2025

I'm getting ../../deps/v8/src/objects/js-temporal-objects.h:27:10: fatal error: 'torque-generated/src/objects/js-temporal-objects-tq.inc' file not found, do we need to add an instruction, in v8.gyp maybe?

@targos
Copy link
Member Author

targos commented Nov 12, 2025

As written in the commit message, this won't work.

@targos
Copy link
Member Author

targos commented Nov 12, 2025

I don't want to be blocking progress on this work, so I'm trying to include the results of my experiments in small steps so that someone else can build on it (and doesn't have to start from scratch)

@aduh95
Copy link
Contributor

aduh95 commented Nov 12, 2025

The commit message makes it sound like the only thing I need to do is to provide a build version of temporal_rs, which is what I've done but I'm getting the error above, so I do think something's missing here. I'm suggesting v8.gyp in case this was somewhat similar to the wasm and icu integration:

'conditions': [
['v8_enable_i18n_support==1', {
'torque_files': [
'<!@pymod_do_main(GN-scraper "<(V8_ROOT)/BUILD.gn" "torque_files =.*?v8_enable_i18n_support.*?torque_files \\+= ")',
],
}],
['v8_enable_webassembly==1', {
'torque_files': [
'<!@pymod_do_main(GN-scraper "<(V8_ROOT)/BUILD.gn" "torque_files =.*?v8_enable_webassembly.*?torque_files \\+= ")',
],
}],
],

@targos
Copy link
Member Author

targos commented Nov 12, 2025

There are many things missing, and I intend to do other PRs after this one. I thought it would be easier to get the changes in small steps 😄.
I don't have enough time to argue on this. If someone wants to bring Temporal in Node.js, feel free to look at the commits I made in https://github.com/targos/node/tree/v8-temporal

@targos targos closed this Nov 12, 2025
@aduh95
Copy link
Contributor

aduh95 commented Nov 12, 2025

FWIW if I apply the following diff:

diff --git a/tools/v8_gypfiles/v8.gyp b/tools/v8_gypfiles/v8.gyp
index c3ec7e3ee92..71d27300d66 100644
--- a/tools/v8_gypfiles/v8.gyp
+++ b/tools/v8_gypfiles/v8.gyp
@@ -27,6 +27,11 @@
           '<!@pymod_do_main(GN-scraper "<(V8_ROOT)/BUILD.gn"  "torque_files =.*?v8_enable_i18n_support.*?torque_files \\+= ")',
         ],
       }],
+      ['v8_enable_temporal_support==1', {
+        'torque_files': [
+          '<!@pymod_do_main(GN-scraper "<(V8_ROOT)/BUILD.gn"  "torque_files =.*?v8_enable_temporal_support.*?torque_files \\+= ")',
+        ],
+      }],
       ['v8_enable_webassembly==1', {
         'torque_files': [
           '<!@pymod_do_main(GN-scraper "<(V8_ROOT)/BUILD.gn"  "torque_files =.*?v8_enable_webassembly.*?torque_files \\+= ")',

I get further, but I'm getting another error: deps/v8/src/objects/js-date-time-format.cc:949:10: error: use of undeclared identifier 'HijriSimulatedMecca'

@targos
Copy link
Member Author

targos commented Nov 12, 2025

https://github.com/targos/node/tree/v8-temporal contains this change and other similar ones.

@legendecas
Copy link
Member

HijriSimulatedMecca was removed in temporal_rs 0.1.1. We should continue use temporal_rs 0.1.0 in this V8 version.

@aduh95
Copy link
Contributor

aduh95 commented Nov 13, 2025

Chromium is already using v0.1.2, I expect V8 will catch up soon if they haven't already: chromium/chromium@f700ec5

EDIT: indeed, v8/v8@0cbecc1

nodejs-github-bot pushed a commit that referenced this pull request Nov 23, 2025
PR-URL: #60703
Refs: #58730
Refs: #60693
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Nov 23, 2025
PR-URL: #60703
Refs: #58730
Refs: #60693
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Nov 23, 2025
PR-URL: #60703
Refs: #58730
Refs: #60693
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Nov 23, 2025
Co-Authored-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: #60703
Refs: #58730
Refs: #60693
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants