module: add module.stripTypeScriptTypes#55282
Conversation
|
Review requested:
|
7c1529b to
ecc1d82
Compare
ecc1d82 to
56fcf17
Compare
|
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 |
56fcf17 to
80fa828
Compare
|
there's also #54250 |
eca9ab2 to
f41d422
Compare
Unfortunately it seems to have been stalled for a while. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
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
🚀 New features to boost your workflow:
|
50afe56 to
9ab643f
Compare
|
I think this could be benefited from a dedicated module |
|
Maybe putting it on module also makes sense? Or module.typescript if we can foresee adding other typescript-related utilities in the future |
I'm ok with a new module, I think parser is ok, I'd like to hear more opinions about it |
|
If we decide to ship it I'll drop the vm commits |
|
If this needs to be a new module, then it should be added to node/lib/internal/bootstrap/realm.js Line 130 in b4e8f1b 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.
|
76ff249 to
4fbcb6c
Compare
|
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 |
anonrig
left a comment
There was a problem hiding this comment.
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/mergehttps://github.com/nodejs/node/actions/runs/11491778677 |
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/mergehttps://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/mergehttps://github.com/nodejs/node/actions/runs/11498595438 |
|
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 $ 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 |
This PR introduces a new api in
node:module.Fixes: #54300