GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — module: refactor ts parser loading by marco-ippolito · Pull Request #54243 · nodejs/node

Conversation

marco-ippolito commented Aug 7, 2024

Copy link

Copy Markdown

Member

This PR refactors the way TypeScript parser is loaded.
Currently the function loadTypeScriptParser is not exposed but could allow overriding the default parsing options, so that an external version of amaro could be used.
It also changes the parse signature to take only the code string input and return { code }.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss and khaosdoctor reacted with rocket emoji

All reactions

🚀 2 reactions


          module: refactor ts parser loading

fa44b7a

nodejs-github-bot commented Aug 7, 2024

Copy link

Copy Markdown

Collaborator

Review requested:

@nodejs/loaders

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot added module

Issues and PRs related to the module subsystem.

needs-ci PRs that need a full CI run. labels

Aug 7, 2024

marco-ippolito added the strip-types Issues and PRs related to TypeScript type stripping. label

Aug 7, 2024

marco-ippolito mentioned this pull request

Aug 7, 2024

Allow TypeScript transpiler customization nodejs/loaders#218

Closed

codecov Bot commented Aug 7, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Codecov Report

Attention: Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 87.11%. Comparing base (e0634f5) to head (fa44b7a).
Report is 459 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/modules/helpers.js	92.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #54243   +/-   ##
=======================================
  Coverage   87.10%   87.11%           
=======================================
  Files         647      647           
  Lines      181739   181758   +19     
  Branches    34887    34887           
=======================================
+ Hits       158310   158332   +22     
- Misses      16738    16740    +2     
+ Partials     6691     6686    -5

Files with missing lines	Coverage Δ
lib/internal/modules/helpers.js	`97.31% <92.00%> (-0.43%)`	⬇️

... and 30 files with indirect coverage changes

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95 approved these changes

Aug 7, 2024

View reviewed changes

marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 7, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 7, 2024

nodejs-github-bot commented Aug 7, 2024

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

marco-ippolito added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label

Aug 7, 2024

nodejs-github-bot commented Aug 7, 2024

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

legendecas commented Aug 7, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

The new function is not exposed as a public function or being able to be used with --expose-internals. How is it supposed to be used to override with an external amaro?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

marco-ippolito commented Aug 7, 2024

Copy link

Copy Markdown

Member Author

The new function is not exposed as a public function or being able to be used with --expose-internals. How is it supposed to be used to override with an external amaro?

it's currently unusable from outside, but it prepares for it eventually

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

legendecas approved these changes

Aug 7, 2024

View reviewed changes

bnb approved these changes

Aug 7, 2024

View reviewed changes

mcollina approved these changes

Aug 7, 2024

View reviewed changes

mcollina left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

nodejs-github-bot commented Aug 7, 2024

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jakecastelli approved these changes

Aug 8, 2024

View reviewed changes

marco-ippolito added the commit-queue PRs queued for automated landing through the Commit Queue. label

Aug 8, 2024

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label

Aug 9, 2024

nodejs-github-bot merged commit 3cbeed8 into nodejs:main

Aug 9, 2024

nodejs-github-bot commented Aug 9, 2024

Copy link

Copy Markdown

Collaborator

Landed in 3cbeed8

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

targos pushed a commit that referenced this pull request

Aug 14, 2024


          module: refactor ts parser loading

1ba2000

PR-URL: #54243
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>

RafaelGSS mentioned this pull request

Aug 19, 2024

v22.7.0 proposal #54452

Merged

targos added the dont-land-on-v20.x label

Sep 21, 2024

Reviewers

mcollina

mcollina approved these changes

bnb

bnb approved these changes

legendecas

legendecas approved these changes

aduh95

aduh95 approved these changes

jakecastelli

jakecastelli approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready

PRs with CI started, the required approvals, and no outstanding review comments.

module

Issues and PRs related to the module subsystem.

needs-ci

PRs that need a full CI run.

strip-types

Issues and PRs related to TypeScript type stripping.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

marco-ippolito commented Aug 7, 2024

Uh oh!

nodejs-github-bot commented Aug 7, 2024

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

nodejs-github-bot commented Aug 7, 2024

Uh oh!

nodejs-github-bot commented Aug 7, 2024

Uh oh!

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

Uh oh!

Uh oh!

marco-ippolito commented Aug 7, 2024

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Aug 7, 2024

Uh oh!

nodejs-github-bot commented Aug 9, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

codecov Bot commented Aug 7, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

legendecas commented Aug 7, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.