Fix type exports and integrate arethetypeswrong

[smaye81]

Our type exports are incorrect for the Node16 module resolution algorithm: When Connect-ES is used with lib checks enabled, TypeScript raises the error TS1479. For a more detailed description, see #679.

This integrates the arethetypeswrong CLI tool for checking types in published packages. It also adds the call to lint the types in CI that it is easy to spot regressions.

This was tested with examples-es using:

• react/esbuild (ESM with esbuild)
• react/rollup (ESM with rollup)
• react/webpack (ESM with webpack)
• react/webpack-cjs (CJS with webpack)

[timostamm]

This was tested with examples-es using:

• react/esbuild (ESM with esbuild)
• react/rollup (ESM with rollup)
• react/webpack (ESM with webpack)
• react/webpack-cjs (CJS with webpack)

That's great, now we know that bundlers work as expected with the changed exports.

But besides bundlers, Node.js and TypeScript also resolve modules. I wonder how well attw guards against mistakes in our exports for the three different consumers. It would be really nice to know what is covered and what isn't. Can we find out?

[timostamm]

Were you able to reproduce #679 and confirm that it's fixed with this?

[smaye81]

This was tested with examples-es using:

• react/esbuild (ESM with esbuild)
• react/rollup (ESM with rollup)
• react/webpack (ESM with webpack)
• react/webpack-cjs (CJS with webpack)

That's great, now we know that bundlers work as expected with the changed exports.

But besides bundlers, Node.js and TypeScript also resolve modules. I wonder how well attw guards against mistakes in our exports for the three different consumers. It would be really nice to know what is covered and what isn't. Can we find out?

I ran some tests on a branch in examples-es that uses just Node by having a single file (main.js) that imports @connectrpc/connect from my local machine (using npm link). I was able to run node main.js and verify that the import was working fine by printing out the imported symbols.

I also did this same test with a file test.ts which imported @connectrpc/connect. I then ran tsc against this file to verify things were working correctly.

I'm not sure if this answers your question or provides sufficient confidence that this works for Node and TS. Let me know though.

[timostamm]

I ran some tests on a branch in examples-es that uses just Node by having a single file (main.js) that imports @connectrpc/connect from my local machine (using npm link). I was able to run node main.js and verify that the import was working fine by printing out the imported symbols.

Nice. I suppose with scripts like these?

t.mjs

import { ConnectError as a} from "@connectrpc/connect";
import { createFetchClient as b } from "@connectrpc/connect/protocol";
import { createTransport as c } from "@connectrpc/connect/protocol-grpc";
import { createTransport as d } from "@connectrpc/connect/protocol-grpc-web";
import { createTransport as e } from "@connectrpc/connect/protocol-connect";
const ok = !!a && !!b && !!c && !!d && !!e;
console.log(ok ? "ok" : "fail")

t.cjs

const { ConnectError: a } = require("@connectrpc/connect");
const { createFetchClient: b } = require("@connectrpc/connect/protocol");
const { createTransport: c } = require("@connectrpc/connect/protocol-grpc");
const { createTransport: d } = require("@connectrpc/connect/protocol-grpc-web");
const { createTransport: e } = require("@connectrpc/connect/protocol-connect");
const ok = !!a && !!b && !!c && !!d && !!e;
console.log(ok ? "ok" : "fail")

Running both scripts with Node.js LTS from v10 to v20 (fnm exec --using=v20 node t.cjs): Only v10 with t.mjs fails, which is expected because this version doesn't support ESM 👍

Now to break it:

$ cd packages/connect
$ npm run build
$ mv protocol.js protocol.js-x
$ npx attw --pack

No error from attw, but now Node.js v10 also fails t.cjs 💥

It's a bummer that attw does not validate these .js exports. They're handwritten and most susceptible to break.

This makes me wonder: Clearly we have to support Node.js v10 module resolution for types, since it's not uncommon to use "moduleResulution": "Node" in a tsconfig. But there is no point in support Node.js v10 itself at runtime - it's been EOL for more than two years.

So I wonder whether we can actually safely delete protocol.js, protocol-connect.js, etc. 🤔
This could potentially break some bundlers, so it's best to keep them for now, and investigate that separately.

I also did this same test with a file test.ts which imported @connectrpc/connect. I then ran tsc against this file to verify things were working correctly.

Seems sufficient 👍

The only open questions from my end are #838 (comment) and #838 (comment).

[smaye81]

Were you able to reproduce #679 and confirm that it's fixed with this?

Whoops, sorry, missed this. Yes, I was able to recreate #679 and can confirm that this fixes that issue.

[timostamm]

LGTM!