module: add module.stripTypeScriptTypes by marco-ippolito · Pull Request #55282 · nodejs/node · GitHub

This PR introduces a new api in node:module.

Review requested:

@nodejs/loaders
@nodejs/vm

I think it makes more sense for it to be just in its own API as a function, which can be used to strip the type before the text is passed into any of the vm.Script/vm.SourceTextModule/vm.compileFunction APIs. That'll also be useful for those who want to transform the text but not compile it at all. The transform option being ignored without the flag could be somewhat surprising, especially paired with cachedData which only has a length check, so it could lead to crashes when V8 tries to reparse the untransformed source for error reporting.

there's also #54250

there's also #54250

Unfortunately it seems to have been stalled for a while.
Anyways good to review, I'm just not sure whether to be strict and throw when mode is strip-only and sourceMap is passed, otherwise will be ignored and could be counter-intuitive

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.42%. Comparing base (7b5d660) to head (8532f1e).
Report is 1390 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #55282   +/-   ##
=======================================
  Coverage   88.41%   88.42%           
=======================================
  Files         653      654    +1     
  Lines      187476   187552   +76     
  Branches    36083    36087    +4     
=======================================
+ Hits       165763   165839   +76     
- Misses      14946    14951    +5     
+ Partials     6767     6762    -5

Files with missing lines	Coverage Δ
lib/internal/main/eval_string.js	80.00% <100.00%> (ø)
lib/internal/modules/cjs/loader.js	97.64% <100.00%> (+0.05%)	⬆️
lib/internal/modules/esm/get_format.js	89.32% <100.00%> (+0.04%)	⬆️
lib/internal/modules/esm/translators.js	93.09% <100.00%> (ø)
lib/internal/modules/helpers.js	98.84% <100.00%> (-0.17%)	⬇️
lib/internal/modules/typescript.js	100.00% <100.00%> (ø)
lib/module.js	100.00% <100.00%> (ø)

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

I think this could be benefited from a dedicated module parser (name bikeshed welcome), instead of vm since the updated PR doesn't involve with any code evaluation. This module could provide additional helpers to address similar needs on manipulating JS source codes.

Maybe putting it on module also makes sense? Or module.typescript if we can foresee adding other typescript-related utilities in the future

I think this could be benefited from a dedicated module parser (name bikeshed welcome), instead of vm since the updated PR doesn't involve with any code evaluation. This module could provide additional helpers to address similar needs on manipulating JS source codes.

I'm ok with a new module, I think parser is ok, I'd like to hear more opinions about it
I wouldnt call it typescript and leave it more generic, maybe someone wants to add ast manipulation or bundling etc... could be the right place

If we decide to ship it I'll drop the vm commits
Pinging TSC as adding a new module seems like a big deal @nodejs/tsc

If this needs to be a new module, then it should be added to

node/lib/internal/bootstrap/realm.js

Line 130 in b4e8f1b

const schemelessBlockList = new SafeSet([

to block require('parser') without the node: prefix, otherwise it breaks https://www.npmjs.com/package/parser or anyone creating an alias of parser in node_modules.

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

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

Looks like this is moving forward, removing TSC agenda lable after discussion in todays meeting, please re-add if there is still something the TSC needs to discuss

Looks like this is moving forward, removing TSC agenda lable after discussion in todays meeting, please re-add if there is still something the TSC needs to discuss

its still blocked by @anonrig review

It was an error on my side. I didn't get the previous notifications and assumed that we're still creating a new module.

Hey, I've read over the past few messages (and the entire issue), and based on them, I've adjusted the labels of this issue, LMK if that was a mistake, as this was on the agenda for TSC, but per the comments above, I've removed it.

edit: I think we simultaneously edited the labels 😅

Commit Queue failed

- Loading data for nodejs/node/pull/55282
✔  Done loading data for nodejs/node/pull/55282
----------------------------------- PR info ------------------------------------
Title      module: add module.stripTypeScriptTypes (#55282)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     marco-ippolito:vm-strip-types -> nodejs:main
Labels     semver-minor, tsc-agenda, needs-ci, review wanted, commit-queue-squash, strip-types
Commits    1
 - module: add module.stripTypeScriptTypes
Committers 1
 - Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/55282
Fixes: https://github.com/nodejs/node/issues/54300
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55282
Fixes: https://github.com/nodejs/node/issues/54300
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 05 Oct 2024 16:28:53 GMT
   ✔  Approvals: 6
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2391013128
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2350567333
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/55282#pullrequestreview-2354092903
   ✔  - Paolo Insogna (@ShogunPanda) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2365865817
   ✔  - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2380232467
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2380688805
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-10-21T17:43:56Z: https://ci.nodejs.org/job/node-test-pull-request/63238/
- Querying data for job/node-test-pull-request/63238/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 55282
fatal: couldn't find remote ref refs/pull/55282/merge

https://github.com/nodejs/node/actions/runs/11491778677

LGTM!

Commit Queue failed

- Loading data for nodejs/node/pull/55282
✔  Done loading data for nodejs/node/pull/55282
----------------------------------- PR info ------------------------------------
Title      module: add module.stripTypeScriptTypes (#55282)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     marco-ippolito:vm-strip-types -> nodejs:main
Labels     semver-minor, needs-ci, commit-queue-squash, strip-types
Commits    1
 - module: add module.stripTypeScriptTypes
Committers 1
 - Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/55282
Fixes: https://github.com/nodejs/node/issues/54300
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55282
Fixes: https://github.com/nodejs/node/issues/54300
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 05 Oct 2024 16:28:53 GMT
   ✔  Approvals: 6
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2391013128
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2350567333
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/55282#pullrequestreview-2354092903
   ✔  - Paolo Insogna (@ShogunPanda) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2391920279
   ✔  - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2380232467
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2380688805
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-10-24T02:58:22Z: https://ci.nodejs.org/job/node-test-pull-request/63238/
- Querying data for job/node-test-pull-request/63238/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 55282
fatal: couldn't find remote ref refs/pull/55282/merge

https://github.com/nodejs/node/actions/runs/11496802024

Commit Queue failed

- Loading data for nodejs/node/pull/55282
✔  Done loading data for nodejs/node/pull/55282
----------------------------------- PR info ------------------------------------
Title      module: add module.stripTypeScriptTypes (#55282)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     marco-ippolito:vm-strip-types -> nodejs:main
Labels     semver-minor, author ready, needs-ci, strip-types
Commits    1
 - module: add module.stripTypeScriptTypes
Committers 1
 - Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/55282
Fixes: https://github.com/nodejs/node/issues/54300
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55282
Fixes: https://github.com/nodejs/node/issues/54300
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 05 Oct 2024 16:28:53 GMT
   ✔  Approvals: 6
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2391013128
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2350567333
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/55282#pullrequestreview-2354092903
   ✔  - Paolo Insogna (@ShogunPanda) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2391920279
   ✔  - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2380232467
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/55282#pullrequestreview-2380688805
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-10-24T09:47:58Z: https://ci.nodejs.org/job/node-test-pull-request/63238/
- Querying data for job/node-test-pull-request/63238/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 55282
fatal: couldn't find remote ref refs/pull/55282/merge

https://github.com/nodejs/node/actions/runs/11498595438

Downloading patch for 55282fatal: couldn't find remote ref refs/pull/55282/merge
same error when trying manually with git node land

Let me try rebase and see if it makes difference

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

Downloading patch for 55282fatal: couldn't find remote ref refs/pull/55282/merge same error when trying manually with git node land

Let me try rebase and see if it makes difference

FWIW Prior to the rebase I was seeing:

$ git ls-remote upstream refs/pull/55282/*
c1c262a134f21bb350e0dfe4c8c50e2009195a04        refs/pull/55282/head
$

which was consistent with the error (no refs/pull/55282/merge). After the rebase:

$ git ls-remote upstream refs/pull/55282/*
8532f1e1e07538901a102c1880700f767d0e6686        refs/pull/55282/head
efb62871253f691abd0ac770e7361f20af13d1d2        refs/pull/55282/merge
$

🤞

Landed in 53b1050

This commit does not land cleanly on v22.x-staging and will need manual backport in case we want it in v22.x.

nodejs-github-bot added module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. vm Issues and PRs related to the vm subsystem. labels Oct 5, 2024

marco-ippolito added the strip-types Issues or PRs related to strip-types support label Oct 5, 2024

marco-ippolito force-pushed the vm-strip-types branch from 7c1529b to ecc1d82 Compare October 5, 2024 16:33

avivkeller reviewed Oct 5, 2024

View reviewed changes

avivkeller added the semver-minor PRs that contain new features and should be released in the next minor version. label Oct 5, 2024

marco-ippolito force-pushed the vm-strip-types branch from ecc1d82 to 56fcf17 Compare October 5, 2024 16:49

marco-ippolito requested review from joyeecheung and legendecas October 5, 2024 17:06

marco-ippolito force-pushed the vm-strip-types branch from 56fcf17 to 80fa828 Compare October 6, 2024 07:18

marco-ippolito changed the title vm: add support for --experimental-strip-types vm: add vm.stripTypeScriptTypes(code, options) Oct 6, 2024

marco-ippolito force-pushed the vm-strip-types branch 2 times, most recently from eca9ab2 to f41d422 Compare October 6, 2024 07:46

marco-ippolito marked this pull request as ready for review October 6, 2024 07:49

atlowChemi reviewed Oct 7, 2024

View reviewed changes

Comment thread lib/vm.js Outdated

Comment thread lib/vm.js Outdated

Comment thread lib/vm.js Outdated

Comment thread lib/vm.js Outdated

marco-ippolito force-pushed the vm-strip-types branch 2 times, most recently from 50afe56 to 9ab643f Compare October 7, 2024 07:27

marco-ippolito changed the title vm: add vm.stripTypeScriptTypes(code, options) lib: add parser.stripTypeScriptTypes Oct 7, 2024

marco-ippolito force-pushed the vm-strip-types branch from 76ff249 to 4fbcb6c Compare October 7, 2024 15:05

marco-ippolito closed this Oct 24, 2024

marco-ippolito reopened this Oct 24, 2024

anonrig approved these changes Oct 24, 2024

View reviewed changes

marco-ippolito added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Oct 24, 2024

avivkeller removed tsc-agenda Issues and PRs to discuss during the meetings of the TSC. review wanted PRs that need reviews. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Oct 24, 2024

ShogunPanda approved these changes Oct 24, 2024

View reviewed changes

module: add module.stripTypeScriptTypes

8532f1e

richardlau approved these changes Oct 24, 2024

View reviewed changes

aduh95 mentioned this pull request Nov 6, 2024

2024-11-11, Version 23.2.0 (Current) #55741

Merged

marco-ippolito mentioned this pull request Dec 10, 2024

[v22.x backport] module: add module.stripTypeScriptTypes #56208

Closed

ruyadorno mentioned this pull request Dec 20, 2024

2025-01-07, Version 22.13.0 'Jod' (LTS) #56329

Merged

marco-ippolito mentioned this pull request Feb 4, 2025

lib: expose tsParse as util.stripTypescriptTypes #54250

Closed

AlCalzone mentioned this pull request May 13, 2026

Consider further Node.js version bumps zwave-js/zwave-js#8820

Closed

coderabbitai Bot mentioned this pull request Aug 16, 2026

feat(opengantry): OpenGantry governance worker (bundle) iii-hq/workers#814

Merged

module: add module.stripTypeScriptTypes by marco-ippolito · Pull Request #55282 · nodejs/node · GitHub

Uh oh!

Uh oh!

module: add module.stripTypeScriptTypes - #55282

module: add module.stripTypeScriptTypes#55282nodejs-github-bot merged 1 commit intonodejs:mainnodejs/node:mainfrom marco-ippolito:vm-strip-typesmarco-ippolito/node:vm-strip-typesCopy head branch name to clipboard

Conversation

marco-ippolito commented Oct 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Oct 5, 2024 • edited by marco-ippolito Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joyeecheung commented Oct 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devsnek commented Oct 6, 2024

Uh oh!

marco-ippolito commented Oct 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Oct 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

legendecas commented Oct 7, 2024

Uh oh!

joyeecheung commented Oct 7, 2024

Uh oh!

marco-ippolito commented Oct 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marco-ippolito commented Oct 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joyeecheung commented Oct 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Oct 21, 2024

Uh oh!

nodejs-github-bot commented Oct 21, 2024

Uh oh!

mhdawson commented Oct 23, 2024

Uh oh!

marco-ippolito commented Oct 23, 2024

Uh oh!

anonrig left a comment

Choose a reason for hiding this comment

Uh oh!

avivkeller commented Oct 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Oct 24, 2024

Uh oh!

ShogunPanda left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Oct 24, 2024

Uh oh!

nodejs-github-bot commented Oct 24, 2024

Uh oh!

marco-ippolito commented Oct 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Oct 24, 2024

Uh oh!

richardlau commented Oct 24, 2024

Uh oh!

nodejs-github-bot commented Oct 24, 2024

Uh oh!

module: add module.stripTypeScriptTypes#55282
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
marco-ippolito:vm-strip-types

marco-ippolito commented Oct 5, 2024 •
edited
Loading

nodejs-github-bot commented Oct 5, 2024 •
edited by marco-ippolito
Loading

joyeecheung commented Oct 5, 2024 •
edited
Loading

marco-ippolito commented Oct 6, 2024 •
edited
Loading

codecov Bot commented Oct 6, 2024 •
edited
Loading

marco-ippolito commented Oct 7, 2024 •
edited
Loading

marco-ippolito commented Oct 7, 2024 •
edited
Loading

joyeecheung commented Oct 7, 2024 •
edited
Loading

avivkeller commented Oct 24, 2024 •
edited
Loading

marco-ippolito commented Oct 24, 2024 •
edited
Loading