build: add ts support in core modules

[marco-ippolito]

Let's try again.
This PR allows Node.js source code to be written in TS.
This is semver major because the build now always requires rust to be installed.
This adds swc as a rust crate dependency and it's only use during build time.
Technically we could replace amaro wasm with this for type stripping but it's not the goal of this PR and in needs other considerations.
I moved an internal from .js and .ts to showcase
Also added a flag so that the transpiled code can be writte on the fs for debugging.
A lot of the addition are vendored crates sorry for the massive PR
I used AI to help me out with things I dont know (rust) so please review carefully
I had to bump the rustc from 1.82 to 1.85, obviously this cannot land if we dont update that on the CI machines nodejs/build#4245

[nodejs-github-bot]

Review requested:

• @nodejs/build
• @nodejs/gyp
• @nodejs/security-wg
• @nodejs/tsc

[anonrig]

@marco-ippolito do you know what steps are needed to run and expose Rust code just like C++?

[marco-ippolito]

@marco-ippolito do you know what steps are needed to run and expose Rust code just like C++?

its already done in this PR, see d9ee95f

basically add the crate in the deps/crates cargo.toml, create a header file if doesnt come with the crate and add it to the node gyp

[ljharb]

Will this type strip at runtime? Or will it do it at build time?

[marco-ippolito]

Will this type strip at runtime? Or will it do it at build time?

Build time

[ljharb]

How does this interact with the --node-builtin-modules-path flag? will it still type strip at runtime in that case?

[marco-ippolito]

How does this interact with the --node-builtin-modules-path flag? will it still type strip at runtime in that case?

Yes, I tested it, it works fine. it plugs in the js2c module

[mcollina]

I'm relatively concerned about adding 1 million and 300 hundred thousand lines of code, even if they are just dependencies.

Is there a solution that does not require this much?

[marco-ippolito]

I dont think so since we are vendoring dependencies. I think the only solution is to have an automation generate them but its kinda impossible to review anyways. (The temporal PR from @legendecas had the same problem)

[legendecas]

I think the question should be is it necessary to have every swc dependencies to be included?

I listed the newly added dependencies, and the followings are the biggest deps that I doubt if they are required:

9.6M	deps/crates/vendor/tracing          => Diagnostic purpose
5.1M	deps/crates/vendor/libc             => Indirect dep of crates `cpufeatures` and `num_cpus`
2.8M	deps/crates/vendor/regex-automata   => Regex engine
1.6M	deps/crates/vendor/bitvec           => Indirect dep of swc_sourcemap
1.6M	deps/crates/vendor/zerocopy         => Indirect dep of hashbrown
1.3M	deps/crates/vendor/zerocopy-derive  => Indirect dep of hashbrown
1.0M	deps/crates/vendor/idna             => Indirect dep of crate `url`

Additionally, crates like wasm-bindgen are not technically needed for the PR purpose.

I think technically many of these deps can be stripped down.

[marco-ippolito]

Can I just like delete them and see if it builds?

[marco-ippolito]

I tried to remove those crates but it seems they are all required to build. Idk if there is a way to know exactly which ones are unused or how to remove them, I'm no rust expert

[targos]

This is blocked by nodejs/build#4245

[ChALkeR]

@marco-ippolito this works on pure v8 cli:

Without bundling, with v8 CLI import()
(which could alternatively be replaced with just concatenating it into the script)

(async function() {
  class TextEncoder {} // only constructed, not actually used
  class TextDecoder {
    constructor(encoding = "utf-8", options = {}) {
      if (encoding !== 'utf-8' || !options.ignoreBOM) throw new Error('Unexpected')
    }
    decode(input = new Uint8Array(), options) {
      if (!(input instanceof Uint8Array) || options) throw new Error('Unexpected')
      return decodeURIComponent(escape(String.fromCharCode.apply(String, input)));
    }
  };

  const Amaro = globalThis.module = {}
  globalThis.require = (arg) => {
    if (arg === 'util') return { TextEncoder, TextDecoder }
    if (arg === 'node:buffer') return {
      Buffer: {
        from: (x, encoding) => {
          if (encoding === 'base64') return Uint8Array.fromBase64(x)
          throw new Error('Unexpected')
        }
      }
    }
    throw new Error('Unexpected')
  }

  await import('./node_modules/amaro/dist/index.js')

  const { code } = Amaro.exports.transformSync("const foo: string = 'bar';", { mode: "strip-only" });
  console.log(code);
})();

chalker@macbook-air _test % ~/.jsvu/bin/v8 tempout.cjs
const foo         = 'bar';

[marco-ippolito]

@marco-ippolito this works on pure v8 cli:

Without bundling, with v8 CLI import().

(async function() {
  class TextEncoder {} // only constructed, not actually used
  class TextDecoder {
    constructor(encoding = "utf-8", options = {}) {
      if (encoding !== 'utf-8' || options.fatal || !options.ignoreBOM) throw new Error('Unexpected')
    }
    decode(input = new Uint8Array(), options) {
      if (!(input instanceof Uint8Array) || options) throw new Error('Unexpected')
      return decodeURIComponent(escape(String.fromCharCode.apply(String, input)));
    }
  };

  const Amaro = globalThis.module = {}
  globalThis.require = (arg) => {
    if (arg === 'util') return { TextEncoder, TextDecoder }
    if (arg === 'node:buffer') return {
      Buffer: {
        from: (x, encoding) => {
          if (encoding === 'base64') return Uint8Array.fromBase64(x)
          throw new Error('Unexpected')
        }
      }
    }
    console.log('arg', arg)
  }

  await import('./node_modules/amaro/dist/index.js')

  var { code } = Amaro.exports.transformSync("const foo: string = 'bar';", { mode: "strip-only" });
  console.log(code);
})();

If we could run this in the js2c.cc, we could skip completely the rust dependency

[mcollina]

Moving to new-only construction of streams, EventEmitters, etc. is one of those "break all the legacy libraries" changes that I somewhat assumed we would never be able to make.

The blast radius of this would be way too great to justify the effort on the ecosystem side. There is a lot of code that depends on this pattern, and updating it all is going to be problematic: even if we deprecate, most developers won't know how to deal with it because it's buried under 10 levels on dependencies on a module that hasn't been updated since 2016. Unless there is a clear reason (like security in the case of Buffer), I would not recommend we do this.

[joyeecheung]

If we could run this in the js2c.cc, we could skip completely the rust dependency

Note that this would not work on cross compiled jobs without setting up emulation - this was the reason why releases on a couple of platforms didn’t have code cache or snapshots, as building those require setting up emulation. Not impossible to do, just needs more work on the build side, though I imagine for experimental architectures like RISC-V it might be even harder.

[joyeecheung]

Moving to new-only construction of streams, EventEmitters, etc. is one of those "break all the legacy libraries" changes that I somewhat assumed we would never be able to make.

Apart from the breakage, until all supported release lines can assume rust toolchain support, moving the files can create a lot of conflicts for backports. e.g. here we changed a .js file to .ts file, it means all subsequent changes to that .ts file on main may require manual backports for releases that do not have support for this, and new files authored in .ts all need manual backports too. If the toolchain requirement needs to be semver-major, it would be safer to only touch existing files until all supported release lines can compile them, and take care with the semverness of all ts-related changes on main.

[ChALkeR]

Not impossible to do, just needs more work on the build side

Still better than importing over a million LoC I think?
Also this can be cross-built (i.e. transformed by v8 built on the host), is that possible to setup?

This just needs v8 to run, anywhere, at build time. Not necessary on target platform.

until all supported release lines can assume rust toolchain suppot

The proposed solution with using v8 + amaro does not need rust toolchain support.
Amaro is already imported and it's wasm.

[marco-ippolito]

Please take a look again, removed the rust dependency, it uses v8 to load amaro and strip types.
Now it 500 LOC so can be reviewed
This could technically be backported to alle release lines (except v20 going eol) because they all have amaro.

[mcollina]

lgtm

[jasnell]

Is this full typescript or type-erasible typescript? I'm +1 on supporting the type-erasible typescript subset but somewhat -1 on using full typescript syntax in core. I don't block if folks really insist on it but I generally believe the erasible syntax approach to be by far the safer option.

[ChALkeR]

It should be noted that unlike TextEncoder API this implementation expects an actual USVString and throws otherwise

While TextEncoder accepts DOMString which it silently converts to USVString

It adds a throw but it's ok for internal usage

[marco-ippolito]

Is this full typescript or type-erasible typescript? I'm +1 on supporting the type-erasible typescript subset but somewhat -1 on using full typescript syntax in core. I don't block if folks really insist on it but I generally believe the erasible syntax approach to be by far the safer option.

Erasable syntax, it will throw on transformation

[Renegade334]

This is unfortunately going down an X-Y problem path.

The objective behind this proposal is to facilitate typechecking in core modules. Absolutely no objection here!

However, the proposed change itself relies on the assumption that the TypeScript language service treats TS the same as JS, just with typechecking. This is not correct.

TypeScript can work with both .js and .ts input, and indeed supports typechecking in both. However, there are some JS constructs that TypeScript treats differently depending on whether it finds them in a .js or a .ts file. Most importantly for us:

• module.exports
  • TL;DR: TypeScript will ignore all exports in .ts files in core
  • in .js files, assignment to module.exports sets the module's top-level export, and assigning properties to module.exports sets individual named exports
  • in .ts files, module.exports is just an arbitrary object with no special meaning: any assignment to module.exports is ignored from an exports perspective
  • TypeScript mandates the use of export = ... in lieu of module.exports = ..., or ESM-style exports for setting individual module.exports properties
  • both of these are transpile-only patterns, and aren't strippable
• require()
  • TL;DR: TypeScript will ignore all imports in .ts files in core, and will disable typechecking on all imported identifiers
  • in .js files, require(...) is considered an import operation:
    • const mod = require(...) will set the type of mod to the value of module.exports in the target module
    • const { a, b, c } = require(...) will set the types of the destructured variables appropriately
  • in .ts files, require(...) is an arbitrary function with no special meaning:
    • const mod = require(...) will not check whether the target module even exists, and the type of mod is the "disable-typechecks" type any
    • const { a, b, c } = require(...) will not check for the existence of module.exports.{a,b,c}, and the destructured variables will all have the "disable typechecks" type any
    • this behaviour is pervasive: any properties of imported variables, any results of calls to imported methods etc. will also have the "disable typechecks" type any
  • this can be worked around by annotating with a "module type" assertion (eg. require(...) as typeof import(...)) – but only if the target is a .js file! (since if it's a .ts file, then it has no exports, due to the first point above)
  • TypeScript mandates the use of import mod = require(...) in lieu of const mod = require(...), or ESM-type imports for importing specific properties
  • both of these are transpile-only patterns, and aren't strippable

Any module in core renamed to .ts essentially "drops out" of the module system as far as the TypeScript language service is concerned. The specific combination of CJS + no transpiled syntax is simply not facilitated in the handling of .ts files.

This can be seen with the colors.ts example in this PR. Consider somewhere that imports this module, eg.

node/lib/internal/assert/assertion_error.js

Line 23 in 7547e79

const colors = require('internal/util/colors');

• When the module is named colors.js, the language service reports the type of colors here to be that of the module's exports, and knows the types of all its properties etc.
• When the module is renamed colors.ts, the type of colors is any, none of its properties are known, and any subsequent typechecking involving colors is disabled.

As previously mentioned, we have access to typechecking with the TypeScript language service in .js files, using the @ts-check directive and comment-style type annotations. This would allow us to achieve the core objective, in a way that is far, far more compatible with core.

[ChALkeR]

Did .cts ever work?
Or does it also require ESM usage and only controls output type?

[Renegade334]

Did .cts ever work? Or does it also require ESM usage and only controls output type?

It's the same deal. It still necessitates import = / export = or ESM-style syntax, and isn't compatible with type stripping.

[ChALkeR]

What benefits are we getting from .ts files for sources? I'm assuming another PR would add typechecks in lint for those?
Perhaps that one could use a transform before passing them to the linter? Would that work?
Likely not for all files though

[marco-ippolito]

Having typescript syntax is worth even if we cannot typecheck everything. @Renegade334 if you feel strongly please request changes.
I'm sure we can work out details in followup, this PR just lays the foundation for this to happen.

[Renegade334]

Having typescript syntax is worth even if we cannot typecheck everything.

The primary goal is the typechecking, not TS syntax in of itself. Renegade334/node@main...typed-core would be the alternative route, providing typechecking via the TypeScript compiler without the pitfalls associated with .ts modules mentioned above.

if you feel strongly please request changes.

I was rather hoping to sway the consensus without having to reach for the red X 🙂