Skip to content

Conversation

@sirtimid
Copy link
Contributor

@sirtimid sirtimid commented Oct 24, 2024

Closes #174

Apparently "moduleResolution": "Node16" sets stricter rules on TS and VScode can't link to the sources, it needs the build files. This PR reverts the default tsconfig to "moduleResolution": "Node" and applies Node16 on build. Unfortunately I've set resolution to Node16 for yarn test:ts (so skipLibCheck works) as with the package reference working TS was asking to include the files from the other packages as well, and since it doesn't allow referencing files while ignoring them during type-checking, it will throw errors if they aren't included.

Also ref: microsoft/TypeScript#40431

@sirtimid sirtimid requested a review from a team as a code owner October 24, 2024 16:39
@sirtimid sirtimid changed the title chore: Fix dev package resolution chore: Configure TypeScript to use source files during development Oct 24, 2024
@sirtimid sirtimid requested a review from rekmarks October 24, 2024 16:40
@sirtimid sirtimid force-pushed the sirtimid/configure-ts-resolution branch from b5a0190 to aa01572 Compare October 24, 2024 16:48
Comment on lines -7 to -8
"module": "Node16",
"moduleResolution": "Node16",
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

lol, reading comprehension fail

@sirtimid sirtimid merged commit 00a2ed4 into main Oct 24, 2024
@sirtimid sirtimid deleted the sirtimid/configure-ts-resolution branch October 24, 2024 16:55
rekmarks added a commit that referenced this pull request Oct 24, 2024
It appears that a better world is possible: we can both use `Node16`
module resolution for builds _and_ test, lint, and develop directly from
our source files.

@sirtimid previously fixed #174 and #181 over several PRs: #192 #193
#194 #195

In #194, we concluded that there was no way to run type checks without
building our source files. As it turns out, this is in fact possible by
disabling [the `composite` config
option](https://www.typescriptlang.org/tsconfig/#composite)[^1]. Having
done this, we now run `lint:ts` as part of our regular lint scripts,
which reliably works without building. In addition, this also fixes type
errors in our `vitest.config.ts` files.

So that we have this documented, let's review our config to understand
why our new config works.

In our root `tsconfig.json` and `tsconfig.package.json` (which
package-level `tsconfig.json` files extend), we set the following:

```json
{
    "module": "Preserve",
    "moduleResolution": "Node10",
}
```

Using [`Node10` for
`moduleResolution`](https://www.typescriptlang.org/tsconfig/#moduleResolution)
causes TypeScript to ignore the `exports` field of `package.json`, which
enables our [`paths`
config](https://www.typescriptlang.org/tsconfig/#paths) in
`tsconfig.packages.json` to correctly resolve our source files. Using
[`Preserve` for
`module`](https://www.typescriptlang.org/tsconfig/#preserve) simply
appears to be compatible with our config. Other options, like `ES2015`,
worked as well, but from the description of the setting it _sounds like_
what we want.

Meanwhile, our `tsconfig.test.json` files have been renamed to
`tsconfig.lint.json`, and they now look like this:

```json
{
  "extends": "./tsconfig.json",
  "compilerOptions": {
    "composite": false,
    "noEmit": true,
    "skipLibCheck": true
  },
  "include": ["./src"],
  "exclude": []
}
```

Due to [the semantics of
`extends`](https://www.typescriptlang.org/tsconfig/#extends),
`references` is omitted, and `include` and `exclude` _overwrite_ their
inherited values. `skipLibCheck` ensures that we ignore errors in
dependencies, and `noEmit` of course ensures that we don't actually
produce any output files.

We still use `Node16` as our `module` and `moduleResolution` options for
actual builds, as is [recommended by
`ts-bridge`](https://ts-bridge.dev/reference/configuration/#node16-module-resolution).
There may come a time when this discrepancy causes trouble for us, but
for now, it appears that we are in a happy place.

[^1]: Boundless gratitude to
microsoft/TypeScript#27069 (comment)
for this insight
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

Successfully merging this pull request may close these issues.

Configure TypeScript to use source files during test and development time

3 participants