module: remove --experimental-transform-types

[marco-ippolito]

This PR removes the --experimental-transform-types flag and all related code, tests, and documentation.

Refs: nodejs/typescript#51

[nodejs-github-bot]

Review requested:

• @nodejs/config
• @nodejs/loaders
• @nodejs/performance
• @nodejs/startup
• @nodejs/typescript

[GeoffreyBooth]

process.features.typescript now only returns a boolean

I think we should add process.features.stripTypes that returns a boolean. This should be a separate PR so that it can be backported.

Then the question becomes what to do with process.features.typescript. I think having a feature called typescript only makes sense if the overall feature has various modes, like currently it's either off (false), strip or transform. But we might someday add other TypeScript-related features like type checking, or tsconfig.json support, or support for various features within tsconfig.json; and these might be individual features to be toggled on or off (like maybe a user enables tsconfig.json paths support but not type checking) and so a single string value for an overall mode doesn't make sense. We could make process.features.typescript a Record<string, boolean> so that there's something like process.features.typescript.strip === true and other features add onto there, or treat each new TypeScript-related feature as a top-level key on process.features.

For the purposes of this PR, perhaps the simplest thing to do would be to just keep process.features.typescript as it is but remove transform as a possible return type. Then a redesign of process.features.typescript can happen in a dedicated PR.

[marco-ippolito]

User can run --no-strip-types so typescript might not be enabled

[aduh95]

• process.features.typescript now only returns a boolean.

I wouldn't do that:

• it's perfectly workable as is, as a string
• if we end up re-introducing --transform-type or another type of typescript handling, it will put us in an awkard position.

Why risk it?

[styfle]

if we end up re-introducing --transform-type or another type of typescript handling, it will put us in an awkard position.

If we think thats a possibility in the future, then we should keep process.features.typescript as a string and also keep the options object in stripTypeScriptTypes() so we can allow strip but throw and error for other modes.

TLDR - keep this semver-major change as breaking only for those using transform but anyone using strip should not have a breaking change. Does that make sense?

[codecov]

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.66%. Comparing base (da5efc4) to head (1708c57).
⚠️ Report is 27 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/modules/typescript.js	75.00%	1 Missing ⚠️
src/compile_cache.cc	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61803      +/-   ##
==========================================
+ Coverage   88.84%   89.66%   +0.82%     
==========================================
  Files         674      676       +2     
  Lines      204957   205951     +994     
  Branches    39309    39434     +125     
==========================================
+ Hits       182087   184670    +2583     
+ Misses      15088    13426    -1662     
- Partials     7782     7855      +73     

Files with missing lines	Coverage Δ
lib/internal/bootstrap/node.js	98.96% <100.00%> (-0.01%)	⬇️
src/compile_cache.h	100.00% <ø> (ø)
src/node_options.cc	76.32% <ø> (-0.05%)	⬇️
lib/internal/modules/typescript.js	92.30% <75.00%> (-5.18%)	⬇️
src/compile_cache.cc	79.09% <0.00%> (-1.64%)	⬇️

... and 210 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.

[GeoffreyBooth]

This PR's goal is to remove transform-types. So in this PR, I think just remove the transform response from process.features.typescript. Any other rethinking or redesign of that property can come in a follow-up PR.

[marco-ippolito]

Applied suggestions, only remove transform related stuff

[ljharb]

LGTM overall

[marco-ippolito]

Cna someone help me understand the CI failure?

[Renegade334]

Cna someone help me understand the CI failure?

The --[experimental-]strip-types CLI flag needs to be reintroduced here:

node/test/parallel/test-runner-cli.js

Lines 67 to 68 in 50a77db

	const args = ['--test', '--test-reporter=tap', '--no-warnings',
	`--test-isolation=${isolation}`];

[aduh95]

Cna someone help me understand the CI failure?

The --experimental-strip-types CLI flag needs to be reintroduced here:

That doesn't really make sense, type stripping has been unflagged for a while, that flag is now a no-op. The issue seems to be that the following branch is no longer needed:

node/test/parallel/test-runner-cli.js

Lines 71 to 74 in 50a77db

	if (!process.config.variables.node_use_amaro) {
	// e.g. Compiled with `--without-amaro`.
	assert.strictEqual(child.status, 1);
	} else {

[Renegade334]

The flag still forces an early exit if no amaro, but if it's not provided, then the default-off behaviour can still be tested.

    assert.strictEqual(child.stderr.toString(), '');
    const stdout = child.stdout.toString();

    assert.match(stdout, /ok 1 - this should pass/);
    assert.match(stdout, /ok 2 - this should pass/);
    assert.match(stdout, /ok 3 - this should pass/);
    if (!process.config.variables.node_use_amaro) {
      // e.g. Compiled with `--without-amaro`.
      assert.doesNotMatch(stdout, /ok 4 - this should pass/);
    } else {
      assert.match(stdout, /ok 4 - this should pass/);
      assert.match(stdout, /ok 5 - this should pass/);
      assert.match(stdout, /ok 6 - this should pass/);
    }

    assert.strictEqual(child.status, 0);
    assert.strictEqual(child.signal, null);

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/71465/

[github-actions]

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @marco-ippolito.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/71468/