microsoft / TypeScript-Website

The Website and web infrastructure for learning TypeScript
https://www.typescriptlang.org
Creative Commons Attribution 4.0 International
2.22k stars 1.37k forks source link

Your Node sample project should include a "lib" field to prevent incorrect recognition of DOM symbols #2991

Open mcclure opened 10 months ago

mcclure commented 10 months ago

You have a github repo https://github.com/microsoft/TypeScript-Node-Starter . This project is the top hit for "Typescript Node sample code" on Google. This repo has multiple issues including a significant misconfiguration in the provided tsconfig.json, and a broken link to the main TypeScript documentation. The repo in question was "archived" one month ago and issues and PRs cannot be filed there.

The instructions for this repo say to file PRs instead of issues for non-critical documentation issues. Although I'm not sure I can honestly call the problems here "critical", I cannot file a PR because, again, the relevant repo is "archived".

Context/relevance

I am here because of this line of code:

const x = name;

Imagine writing a TypeScript program targeting Node, containing this line but not defining "name" first. TypeScript will not find an error. I encountered this in a project based on this one: https://github.com/mcclure/ts-hello-cmdline … but with TypeScript upgraded to newest 5.x (version is irrelevant here). (Can provide exact sample code on demand.) I was very, very surprised to find this program typechecking (because it uses an undefined symbol) and initially suspected I had misconfigured tsc. For this reason, I checked Google for Node sample code. I was happy to find the https://github.com/microsoft/TypeScript-Node-Starter project, which I assumed would be of high reliability due to being an official Microsoft repo. I found no differences between its configuration and mine, except the inclusion of "strict":true, which did not affect the problem line.

With the help of some nice people on "Mastodon", I learned the following:

Having learned this, I immediately set out to make a PR for the TypeScript-Node-Starter repo adding the lib field so the next person to have this problem would have a better experience. As mentioned, this is not currently possible.

There is a second problem

Once I realized the TypeScript-Node-Starter repo was locked, I started examining the README to see if there was somewhere else I should be looking. For example when the https://github.com/microsoft/TypeScript-Handbook repo was closed there was a helpful, prominent note added to the repo description redirecting people to this repo here (TypeScript-Website). For TypeScript-Node-Starter there isn't, but there is this disclaimer at the top:

It is not a goal to be a comprehensive and definitive guide to making a TypeScript and Node project, but as a working reference maintained by the community. If you are interested in starting a new TypeScript project - check out the bootstrapping tools reference in the TypeScript Website

Perhaps I would find more at that link? In fact, the link in that paragraph is broken. If you go there, your browser will enter an infinite loop of redirecting to https://www.typescriptlang.org/docs/home, a blank white page consisting solely of a redirect to https://www.typescriptlang.org/docs/home. This is less helpful than it could be.

To summarize, I conclude there are three separate problems:

  1. Your Node sample code is not adequately configured for Node, and the misconfiguration can cause subtle and frustrating problems of the exact type TypeScript exists to fix.
  2. The link from your Node sample code to the main documentation is out of date.
  3. Your website has a weird URL https://www.typescriptlang.org/docs/home.html that contains a faulty redirect to itself.

Additional notes

* On Bing, if you're wondering, it is the third hit. The first Bing hit is a page on NodeJS's website (the NodeJS document does not say anything about tsconfig lib) and the second is a tutorial posted in DigitalOcean's support pages (it suggests using "lib": ["es2015"]).

DanielRosenwasser commented 10 months ago

The TL;DR is that the doc issues are easy to fix, the rest may not be. I can try to do this a little later this week. Here's the context.

The repo has actually been archived for the last few years, and unfortunately I believe it was only un-archived due to some weird requirements around updating dependencies. We should more explicitly call out the archive status

I am here because of this line of code:

const x = name;

I don't see this line in the repo, so I'm guessing you hit this issue in your own code (it's a major footgun DOM API, and similar issues happen with print for newcomers). One of the issues with the sample is that it predates things like project references which made it more reasonable to structure a codebase like

.
├── client
│   ├── tsconfig.json (uses `"lib": ["dom", "es2020"]` and `types: []`)
│   └── src
│       └── ...
├── server
│   ├── tsconfig.json (uses `"lib": ["es2020"]` and `types: ["node"]`)
│   └── src
│       └── ...
└── shared
    ├── tsconfig.json
    └── src
        └── ...

(the types option controls auto-inclusion of @types packages like @types/node)

So the codebase is written with a mixture of Node and client code under a single tsconfig.json and hopes for the best. It's not ideal, but my guess was that best practices were harder at the time, and that structuring was "good enough" for a lot of people (it probably still is, though I wouldn't do that myself either).

mcclure commented 10 months ago

@DanielRosenwasser, thanks.

I don't see this line in the repo, so I'm guessing you hit this issue in your own code

Yes, sorry. I can show you the code where I hit the issue if it helps, but the code isn't the point, the point is "I needed to set lib, and your sample code was unhelpful in showing me how to do so".

So the codebase is written with a mixture of Node and client code under a single tsconfig.json and hopes for the best.

Okay, I don't think that was clear.

I'm not necessarily saying you need to modernize the sample. What I am saying is if you aren't modernizing the sample, you should clearly document the sample (in the README, if not the top-of-page repo description) as being out-of-date, and link to some superior documentation/sample code (for the specific case of using TypeScript with node).

In the day since I filed this bug, I have realized you actually do have some superior documentation. Already. I found this page:

https://github.com/microsoft/TypeScript/wiki/Node-Target-Mapping

Although this page doesn't provide everything someone needs needs to get going with TypeScript+Node, it does answer the precise question I was looking for when I got honeypotted by Typescript-Node-Starter and its excellent SEO: "What fields should my tsconfig.json have when running Node?".

andrewbranch commented 10 months ago

TIL about https://github.com/microsoft/TypeScript/wiki/Node-Target-Mapping. Please ignore the module setting in all of those examples. module should always be set to node16 or nodenext when targeting Node.js. Going to go update that wiki page now.