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

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

Open
mcclure opened this issue Dec 5, 2023 · 3 comments

Comments

@mcclure
Copy link

mcclure commented Dec 5, 2023

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:

  • The code is, unfortunately, in some sense "correct" because there is (or was, I was told it was deprecated) a name field on document in the w3 DOM standard, and therefore the symbol is in lib.dom.d.ts. (I made some effort to find this field in the standard and failed. name turns out to be a difficult word to search for. Maybe in the current standard it's just been removed.)
  • The code should still be treated as invalid in my situation because Node, being a non-web host environment, does not have the DOM. Running node at the command line and typing symbols like name, document or window gives you an unknown reference error. But! It turns out the reason TypeScript is treating my Node code as if DOM symbols were in scope is because I was not correctly using the lib configuration option in tsconfig (doc here) which exists for just this purpose.
  • Adding "lib": ["es2020"], to my tsconfig.json fixes the problem and causes name to correctly be flagged as an undefined variable, without losing access to other ecmascript standard symbols like Set.

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

  • I am actually not entirely sure "lib": ["es2020"], is the correct version of the ECMAScript library to opt into. One assumes different versions of Node will target different ES standard levels. The correct "lib" for a given Node version might in fact be some complex construction like ["es2016", "ES2019.Object", ES2019.String"] or I don't know what.. This is a point on which documentation or best-practices recommendations from TypeScript and/or Microsoft— for example in your highly-visible TypeScript-Node-Start project— would be very useful.
  • My "problem 3" above actually I could theoretically file a PR on. I made an attempt to do so, but hit a dead end quickly. Searching the repo for "home.html", I find this file cataloguing the typescriptlang site's redirects, and I find /docs/home.html redirects to /docs/home and /docs/home redirects to /docs. The code looks correct and the problem appears to require more familiarity with your site code than I have. If it helps, we did find https://www.typescriptlang.org/docs/home/— in other words, the Weird URL but with a / at the end— correctly redirects to docs/.
  • You might reply to this issue by saying that Node is not a Microsoft product and/or that TypeScript-Node-Starter is not an official/supported repo. If this happens to be the response, I want to repeat this repo is the top hit on Google* for "TypeScript Node sample project", and as a repo posted by the Microsoft account on Microsoft's code hosting site (GitHub) it presents the appearance of being very official. I would say it is a useful resource worth keeping maintained.

* 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
Copy link
Member

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
Copy link
Author

mcclure commented Dec 6, 2023

@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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants