Skip to content

tools: rewrite js2c.cc in rust#62565

Open
anonrig wants to merge 1 commit intonodejs:mainfrom
anonrig:yagiz/rewrite-js2c
Open

tools: rewrite js2c.cc in rust#62565
anonrig wants to merge 1 commit intonodejs:mainfrom
anonrig:yagiz/rewrite-js2c

Conversation

@anonrig
Copy link
Copy Markdown
Member

@anonrig anonrig commented Apr 2, 2026

I wanted to see how adding more Rust would work with the existing toolchain & tools. There is no specific reason for this rewrite that can be justified other than the benefits of Rust over C++.

The code is a lot simpler and a lot readable now (as far as I can tell)

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/loaders
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 2, 2026
@anonrig anonrig force-pushed the yagiz/rewrite-js2c branch from 9f06b3e to 29fad45 Compare April 2, 2026 17:04
@anonrig anonrig requested review from aduh95 and joyeecheung April 2, 2026 17:05
@joyeecheung
Copy link
Copy Markdown
Member

joyeecheung commented Apr 2, 2026

I think for the code of js2c per-se moving to rust would be a fun exercise though most of the Rust over C++ benefit doesn't really apply for something that's just a build tool (if it weren't for the slowness, we could've stayed in Python). But taking build complexity into consideration it's not very worth it IMO e.g. it's only easy when js2c has no extra dependencies, but for changes like #62146 (which makes js2c link to V8) this would add more obstacles.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.71%. Comparing base (a9ac9b1) to head (29fad45).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main   #62565    +/-   ##
========================================
  Coverage   89.70%   89.71%            
========================================
  Files         695      695            
  Lines      214213   214417   +204     
  Branches    41021    41065    +44     
========================================
+ Hits       192167   192354   +187     
  Misses      14113    14113            
- Partials     7933     7950    +17     
Files with missing lines Coverage Δ
lib/internal/bootstrap/realm.js 96.21% <100.00%> (ø)
src/node_builtins.h 100.00% <ø> (ø)

... and 51 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Qard
Copy link
Copy Markdown
Member

Qard commented Apr 2, 2026

but for changes like #62146 (which makes js2c link to V8) this would add more obstacles.

The type-stripping itself is in Rust though, so that could just be plugged into a Rust version of js2c fairly easily and do translation for core TS files at build time. Might actually be a good thing to have this in Rust.

@marco-ippolito
Copy link
Copy Markdown
Member

marco-ippolito commented Apr 2, 2026

but for changes like #62146 (which makes js2c link to V8) this would add more obstacles.

The type-stripping itself is in Rust though, so that could just be plugged into a Rust version of js2c fairly easily and do translation for core TS files at build time. Might actually be a good thing to have this in Rust.

not really, it's a wasm. the problem of adding rust dependencies is vendoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants