Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: recommend setting noEmit: true in tsconfig.json #57320

Merged
merged 2 commits into from
Mar 6, 2025

Conversation

styfle
Copy link
Member

@styfle styfle commented Mar 4, 2025

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/typescript

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Mar 4, 2025
@styfle styfle requested a review from jakebailey March 4, 2025 22:28
@@ -81,6 +81,7 @@ but we recommend version 5.8 or newer with the following `tsconfig.json` setting
```json
{
"compilerOptions": {
"noEmit": true, // Optional - prevent tsc from emitting transpiled JS
Copy link
Contributor

Choose a reason for hiding this comment

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

The current wording is a literal description of the tsconfig flag - which can be independently googled. I think the added value we can bring here in the docs is to explain why you might want to use it. For example...

Suggested change
"noEmit": true, // Optional - prevent tsc from emitting transpiled JS
"noEmit": true, // Optional - see Note (1)
...
Note (1): `"noEmit"` is useful if your code is only intended to be consumed and executed as `*.ts` files. If instead you need to produce `*.js` files for performance or distribution reasons, you won't need this flag.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a variation of this in 839322d

@styfle styfle requested a review from robpalme March 6, 2025 16:21
@@ -90,6 +91,10 @@ but we recommend version 5.8 or newer with the following `tsconfig.json` setting
}
```

> \[!NOTE]
> Use the `noEmit` option if you intend to only execute `*.ts` files, for example a build script.
> You won't need this flag if you intend to distribute `*.js` files for performance reasons.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
> You won't need this flag if you intend to distribute `*.js` files for performance reasons.
> You won't need this flag if you intend to distribute `*.js` files.

for whatever reason like npm package

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good to me. I created a new PR since this one already merged.

@marco-ippolito marco-ippolito added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Mar 6, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 6, 2025
@nodejs-github-bot nodejs-github-bot merged commit 5401ac6 into nodejs:main Mar 6, 2025
26 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 5401ac6

@styfle styfle deleted the styfle/issue-57294 branch March 6, 2025 23:34
aduh95 pushed a commit that referenced this pull request Mar 9, 2025
PR-URL: #57320
Fixes: #57294
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
aduh95 pushed a commit that referenced this pull request Mar 9, 2025
PR-URL: #57320
Fixes: #57294
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing noEmit: true from default tsconfig.json in type stripping docs
6 participants