[v22.x backport] module: unflag --experimental-strip-types

[marco-ippolito]

PR-URL: #56350
Fixes: nodejs/typescript#17
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Mohammed Keyvanzadeh mohammadkeyvanzade94@gmail.com
Reviewed-By: Paolo Insogna paolo@cowtech.it
Reviewed-By: Pietro Marchini pietro.marchini94@gmail.com

Just making sure no blockers
It requires #57121 to be landed together since it contains an important fix, and I want to be as stable as possible before landing.
Technically #57121 needs to land in v23 first and wait two weeks.
Next v23 is planned for 2025-03-11 and the next v22 is 2025-03-18 which is not enough time between the two.
So probably needs to wait 2025-04-15

@nodejs/tsc for fyi

I don't think we caused any breakage when unflagging in v23.

[nodejs-github-bot]

Review requested:

• @nodejs/config
• @nodejs/performance
• @nodejs/test_runner
• @nodejs/typescript

[mcollina]

lgtm

[marco-ippolito]

Issue with test runner #56546

[B4nan]

I still haven't had the time to create an isolated repro, but the type stripping feature broke things for us, executing compiled code via tsc no longer works on node 23 unless I explicitly disable the type stripping. We use experimentalDecorators and compile to ESM in there if that matters. The problem is triggered when dynamically importing the compiled JS files (so no TS involved there). Already mentioned in #54901 (comment), the repro is technically there, but not really minimal. I will try to find some time to get an isolated reproduction soon.

[marco-ippolito]

I still haven't had the time to create an isolated repro, but the type stripping feature broke things for us, executing compiled code via tsc no longer works on node 23 unless I explicitly disable the type stripping. We use experimentalDecorators and compile to ESM in there if that matters. The problem is triggered when dynamically importing the compiled JS files (so no TS involved there). Already mentioned in #54901 (comment), the repro is technically there, but not really minimal. I will try to find some time to get an isolated reproduction soon.

please open an issue with a minimal repro, it's very hard to tell whats going on
I see you have decorators enabled 🫤

[JakobJingleheimer]

I still haven't had the time to create an isolated repro, but the type stripping feature broke things for us, executing compiled code via tsc no longer works on node 23 unless I explicitly disable the type stripping. We use experimentalDecorators and compile to ESM in there if that matters. The problem is triggered when dynamically importing the compiled JS files (so no TS involved there). Already mentioned in #54901 (comment), the repro is technically there, but not really minimal. I will try to find some time to get an isolated reproduction soon.

Enabling the new erasableSyntaxOnly (introduced in 5.8) in tsconfig may help you pinpoint the cause.

[B4nan]

Enabling the new erasableSyntaxOnly (introduced in 5.8) in tsconfig may help you pinpoint the cause.

My issue is running a compiled code (via tsc). It's already JS, I don't see how this would help me. I do use stuff that erasableSyntaxOnly disallows, I do not plan to use the native TS support in node, what I care about is being able to still use node on the compiled output from tsc, that is what broke.

I already found the part that breaks, it is an optional symbol property, it's not about decorators.

const OptionalProps = Symbol('OptionalProps');

abstract class BaseEntity<Optional = never> {
    [OptionalProps]?: 'createdAt' | 'updatedAt' | Optional;
}

This gets compiled (by tsc) into:

const OptionalProps = Symbol('OptionalProps');

class BaseEntity {
    [OptionalProps]; // still here and breaks when running via `node` 23, works if type stripping is disabled
}

This is happening when dynamically importing this compiled JS file. The code is clearly valid JS, I can run it in node as well as browsers.

I will try to wrap this up in the evening and create the issue. Sorry for the noise here, but this will affect pretty much all of our users, so enabling this flag is quite a BC for us.

[marco-ippolito]

I found your error:

// node_modules/@mikro-orm/core/metadata/MetadataDiscovery.js
// PATH: /Users/marcoippolito/Documents/projects/test/guide/src/modules/common/base.entity.ts NAME: Base
 const targets = await this.getEntityClassOrSchema(path, name);

You are doing a dynamic import of ts with decorators.
base.entity.ts has decorators inside, there is definitely a bug in the logic of your application in

All files you are matching:

filepath [
  './src/modules/common/base.entity.ts',
  './src/modules/article/article-listing.entity.ts',
  './src/modules/article/article.entity.ts',
  './src/modules/article/comment.entity.ts',
  './src/modules/article/tag.entity.ts',
  './src/modules/user/user.entity.ts'
]

Without experimental-strip-types you are matching:

filepath [
  './dist/modules/article/article-listing.entity.js',
  './dist/modules/article/article.entity.js',
  './dist/modules/article/comment.entity.js',
  './dist/modules/article/tag.entity.js',
  './dist/modules/common/base.entity.js',
  './dist/modules/user/user.entity.js'
]

I think your issue is detectTsNode is set to true, so it will use the .ts glob to find files.

 static detectTsNode() {
   /* istanbul ignore next */
   return process.argv[0].endsWith('ts-node') // running via ts-node directly
       // @ts-ignore
       || !!process[Symbol.for('ts-node.register.instance')] // check if internal ts-node symbol exists
       || !!process.env.TS_JEST // check if ts-jest is used (works only with v27.0.4+)
       || !!process.env.VITEST // check if vitest is used
       || !!process.versions.bun // check if bun is used
       || process.argv.slice(1).some(arg => arg.includes('ts-node')) // registering ts-node runner
       || process.execArgv.some(arg => arg === 'ts-node/esm') // check for ts-node/esm module loader
       || (require.extensions && !!require.extensions['.ts']); // check if the extension is registered
    }

|| (require.extensions && !!require.extensions['.ts']); // check if the extension is registered

There we go, this is your bug, .ts is registered by type-stripping which behaves differently from ts-node.
You really should not rely on a deprecated property since v0.10.6 😞