Skip to content

Add type checking via Deno#178

Open
stefnotch wants to merge 19 commits into
mainfrom
deno-tsc
Open

Add type checking via Deno#178
stefnotch wants to merge 19 commits into
mainfrom
deno-tsc

Conversation

@stefnotch
Copy link
Copy Markdown
Contributor

Should be usable now :)
For easier merging, please merge #177 first. Or I could rebase this.

@github-project-automation github-project-automation Bot moved this to Unprioritized in wesl-js Jul 12, 2025
@mighdoll
Copy link
Copy Markdown
Contributor

It'd be fun to get used to reliable find references again. A bit of extra deno specific lint and a couple of deno.json files looks like a manageable overhead to give that a try...
But can we convince deno to ignore the tsconfig files rather than deleting them? Modest overhead to add deno as an addition seems reasonable, but deno shouldn't force us off tsconfig reading tools. I'm annoyed with upstream deno for creating this problem, it's not to their benefit to be incompatible with mainstream ecosystem.

@stefnotch
Copy link
Copy Markdown
Contributor Author

Sure, I'll it to ignore tsconfig files instead of deleting them.

Though, which of our tools still use the tsconfig files?

  • oxc: Reads paths and project references. We aren't using those features.
  • biome: Could not find anything in the documentation
  • tsc and tsgo: Deno runs tsc. Do we need both?
  • dts file generation: We have both vite-plugin-dts and tsdown. I'll have to dig in into how they generate the dts files. I think tsdown can use oxc when isolatedDeclarations is enabled, and completely skip the Typescript compiler.

@stefnotch
Copy link
Copy Markdown
Contributor Author

Upon some further investigation: It isn't Deno that was being weird with the tsconfig.json files.
Instead our tsconfig setup is broken. It always assumes that it has access to Node.js types, even when they clearly shouldn't be used.

For example, I can put the following in a mini-parse/src/examples example

let a: typeof process | null = null;

and it passes with zero complaints. But mini-parse should be type-checked with tsconfig.base.json, which doesn't have access to the Node.js APIs.

@stefnotch
Copy link
Copy Markdown
Contributor Author

stefnotch commented Jul 13, 2025

Apparently our base tsconfig should explicitly specify the types
https://www.typescriptlang.org/tsconfig/#types

(Edit: Oh god npm is so unsafe. When a dependency of a dependency happens to depend on @types, it'll still use the flat layout. And then Typescript will automatically use that. So out of the box, if anything in your dependency tree depends on @types/node, then you get that.
pnpm fixes even more than I thought xD)

@mighdoll
Copy link
Copy Markdown
Contributor

  • tsc and tsgo: Deno runs tsc. Do we need both?
    I run them from the command line in debug situations, e.g. --trace-resolution. Also we'll need tsc/tsgo to keep checking things that deno doesn't cover.
    more generally, let's not let Deno drag us Deno island, unable to drop in the latest new tool meant for the mainstream ecosystem.
    I played a bit with running benchmarks on bun (on the idea that testing for JSC might prove useful), reads tsconfig. vitest used to read tsconfig, tsgo is obviously going to be adding new features, etc.

@stefnotch
Copy link
Copy Markdown
Contributor Author

Reasonable answer. Then I'll tweak our setup to fix the Typescript thingy, and then Deno should also start doing the right thing.

@stefnotch
Copy link
Copy Markdown
Contributor Author

Okay, it's been reconfigured to work with the tsconfig.json files. All type checkers succeed, but there's still some weirdness like

image

@stefnotch
Copy link
Copy Markdown
Contributor Author

Next Deno update is about to ship, and it'll have some tsconfig improvements. Their native tsconfig support is apparently very new.
So I'll wait a few more days before updating this PR again.

Comment thread tools/tsconfig.node.json Outdated
@stefnotch
Copy link
Copy Markdown
Contributor Author

After merging this, I wholeheartedly recommend checking out deno doc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Unprioritized

Development

Successfully merging this pull request may close these issues.

2 participants