fails-components / webtransport

Http/3 webtransport support for node
Other
132 stars 20 forks source link

fix(types): fix typescript types not being found #300

Closed mikavilpas closed 4 weeks ago

mikavilpas commented 1 month ago

Hi, I'm working on an exploration project about new things, and ran into the following issue:

image

This commit adds the missing types to the package.json file so that typescript can find them. This fixes the following error:

[Running: npx tsc]
server/server.ts:7:29 - error TS7016: Could not find a declaration file for module '@fails-components/webtransport'. '/Users/mikavilpas/git/webtransport/main/lib/index.node.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/fails-components__webtransport` if it exists or add a new declaration (.d.ts) file containing `declare module '@fails-components/webtransport';`

7 import { Http3Server } from "@fails-components/webtransport"
                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~`
mikavilpas commented 1 month ago

I might be able to provide additional things. Let me know if you'd prefer

martenrichter commented 1 month ago

Well, another contributor added at least a check that the compilation works. And there were some issues about the types on the current setup before (https://github.com/fails-components/webtransport/issues/233), that were solved, so I am wondering if there is really an issue. (But I am not using typescript in my projects, which makes types always a weak point.)

mikavilpas commented 1 month ago

Ok, sounds like the current situation is unclear. If it seems to be working for others, I'm in a position in which it's difficult to argue, but I have an idea that could be tried out:

I found this tool just now that claims to be able to check if the types are "wrong":

It can also be run on the command line. Here are the results after my current changes for /main:

image

As you can see, it seems that ESM now works with some fallback (🐛 worm mode in the picture 😄), but it still fails the check (🔴 exit code 1 is visible at the bottom).


To summarize, here are some suggestions:

  1. Maybe this tool could be used to check that everything is in working order
  2. A CI pipeline could be built that uses the CLI version of the tool to perform this check in the future
  3. An example project could be added, perhaps somewhere inside the monorepo, that could test type checking (or simple runtime behaviour)
martenrichter commented 1 month ago

Maybe @achingbrain, who brought the initial type of support (https://github.com/fails-components/webtransport/issues/93), can say something about the change? In contrast to me, he knows about types.

I do not know how good the tool is. It is very unlikely that I will touch the current project's structure soon, as it is already suitable for many different transports.

mikavilpas commented 1 month ago

@achingbrain are you aware of a project I could try out? I could use a working example.

achingbrain commented 1 month ago

This change is necessary to use the newer “node16”/“nodenext” moduleResolution values. The only change I would make is that the TS docs say that the “types” entry should be first in the exports map values: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-7.html#packagejson-exports-imports-and-self-referencing

martenrichter commented 4 weeks ago

@mikavilpas If you made the change suggested by @achingbrain, I will merge the PR. A release will follow next weekend at the latest. I do not think that automated tests are necessary.

mikavilpas commented 4 weeks ago

Awesome 👍🏻

The types entries now precede the import entries. Thanks for the prompt response!