From 05db46b5db5df01196dd481143ad132407df58ea Mon Sep 17 00:00:00 2001 From: reillylm Date: Fri, 24 Jan 2025 17:41:46 -0500 Subject: [PATCH 1/2] prevent third-party types in twoslash snippets **Overview** In the React code for the site, we're using third-party types from the `@types/*` packages. This is good, but it seems that these types are unintentionally also getting included as part of the compilation of our twoslash snippets in our docs pages. This change makes it so that these types are _not_ included in our twoslash snippets by default. We can still get them if we need them in a particular snippet, by specifying the `@types` compiler option within the snippet itself. In the current docs, though, we don't need them most of the time; I counted only 2 snippets where this is necessary (out of the ~800 we produce during the Gatsby build). This general change started when I noticed a [specific issue on the "Object Types" page](https://www.typescriptlang.org/docs/handbook/2/objects.html#the-array-type) (more detail below). In addition to fixing that specific issue, this change offers a slight "correctness" improvement where we use the `console` API in our snippets, since our "quick info" boxes will now show types from the default TS DOM lib, rather than ones from `@types/node`. This change also offers a performance improvement of the Gatsby build; here are some times from my machine (running `gatsby clean` and deleting the twoslash cache directory between runs): ``` // before $ time pnpm build-site 591.06s user 82.26s system 269% cpu 4:10.10 total // after $ time pnpm build-site 333.84s user 41.71s system 307% cpu 2:02.18 total ``` Since this change is only to our Gatsby config, it shouldn't affect the Playground (it will actually make our docs snippets more consistent with the Playground, since we don't bring in third-party types by default there). **Background** In ["Object Types"](https://www.typescriptlang.org/docs/handbook/2/objects.html#the-array-type), we demonstrate what the `Array` type looks like in the default TS lib by defining `interface Array` in a snippet. However, we have the unexpected errors TS2317 and TS2428, which have to do with incorrectly extending an existing interface. Weirdly, if we open the corresponding Playground link, the Playground doesn't show these errors. What's going on? It turns out that, despite our specifying `@noLib: true` in the snippet, when we compile with twoslash, we're "accidentally" ending up with `@types/node` in the compilation, because it's in our own `node_modules` directory. This explains why we don't see the errors in the Playground: there, we use the "in-memory filesystem" provided by `createSystem` in `typescript-vfs`, but during the Gatsby build, we use the `createFSBackedSystem`, who delegates to the real filesystem, so that when TS goes looking for `node_modules/@types` [per the default `typeRoots` behavior](https://www.typescriptlang.org/tsconfig/#typeRoots), it will discover the one from our own project. Specifically, our errors are because `@types/node` contains a type definition for `Array`, but in our snippet, we try to define `Array` with a differently-named type parameter. Therefore, we _could_ work around the error by simply changing `Type` to `T`. However, since our intention with `@noLib: true` was that `Array` would have no other declarations, the current behavior seems wrong, and therefore we should find a better solution. It actually gets a little weirder, since `@types/node` contains a triple-slash reference to the default ES2020 lib, so now we're _really_ getting the lib who we asked not to get :) furthermore, it's the ES2020 lib, even though our `target` is set to ES2016 in the twoslash default options. I discovered this behavior by stepping through the twoslash logic in the debugger and inspecting things like: ``` ls.getProgram().getSourceFiles() ls.getProgram().getAutomaticTypeDirectiveNames() ls.getTypeDefinitionAtPosition('', ) ``` **What docs actually change with this change** I did diffs of the Gatsby output `public` directory to get a concrete answer to this. I found only 2 places where losing third-party types results in a "decrease in correctness," which we can fix by specifying `@types` in the snippet. Then there's the aforementioned change to use the `console` types from the DOM lib (either an "increase in correctness" or a no-op :)). Like mentioned above, our "accidental" inclusion of `@types/node` resulted in the transitive inclusion of the ES2020 lib. When we lose this, we fall back to our explicitly-configured `target` of ES2016. This would be a problem for snippets who use APIs from those newer ES versions; seems like there's only 1 place where this happens, though: in ["Basics"](https://www.typescriptlang.org/docs/handbook/2/basic-types.html#non-exception-failures), when we use `toLocaleLowerCase`: ``` // from lib/lib.es5.d.ts toLocaleLowerCase(locales?: string | string[]): string; // from lib/lib.es2020.string.d.ts toLocaleLowerCase(locales?: Intl.LocalesArgument): string; ``` Since we were getting ES2020 without knowing it anyway, we might as well make it our default `target`. --- packages/documentation/copy/en/javascript/JSDoc Reference.md | 2 ++ packages/typescriptlang-org/gatsby-config.js | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/packages/documentation/copy/en/javascript/JSDoc Reference.md b/packages/documentation/copy/en/javascript/JSDoc Reference.md index 7cd5d7b23e06..669834263f90 100644 --- a/packages/documentation/copy/en/javascript/JSDoc Reference.md +++ b/packages/documentation/copy/en/javascript/JSDoc Reference.md @@ -221,6 +221,7 @@ myPet.name; import types can be used to get the type of a value from a module if you don't know the type, or if it has a large type that is annoying to type: ```js twoslash +// @types: node // @filename: accounts.d.ts export const userAccount = { name: "Name", @@ -746,6 +747,7 @@ Otherwise, `@example` will be parsed as a new tag. ### Other supported patterns ```js twoslash +// @types: react class Foo {} // ---cut--- var someObj = { diff --git a/packages/typescriptlang-org/gatsby-config.js b/packages/typescriptlang-org/gatsby-config.js index 6ca35392c035..4b7ea0731eb8 100644 --- a/packages/typescriptlang-org/gatsby-config.js +++ b/packages/typescriptlang-org/gatsby-config.js @@ -158,6 +158,10 @@ module.exports = { defaultOptions: { noErrorValidation: true, }, + defaultCompilerOptions: { + types: [], + target: 7, // ES2020 + }, }, }, "gatsby-remark-copy-linked-files", From 813e9b779b0416a0db4ad4b5bd10f93a4ed86817 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Sat, 25 Jan 2025 21:11:17 -0800 Subject: [PATCH 2/2] require TS in gatsby-config --- packages/typescriptlang-org/gatsby-config.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/typescriptlang-org/gatsby-config.js b/packages/typescriptlang-org/gatsby-config.js index 4b7ea0731eb8..1322dfda213a 100644 --- a/packages/typescriptlang-org/gatsby-config.js +++ b/packages/typescriptlang-org/gatsby-config.js @@ -14,6 +14,8 @@ if (process.env.BOOTSTRAPPING) { require("./scripts/ensureDepsAreBuilt") +const ts = require("typescript"); + // https://github.com/gatsbyjs/gatsby/issues/1457 require("ts-node").register({ files: true }) const { join } = require("path") @@ -160,7 +162,7 @@ module.exports = { }, defaultCompilerOptions: { types: [], - target: 7, // ES2020 + target: ts.ScriptTarget.ES2020, }, }, },