-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The Types Are Wrong! #982
Comments
Motivation: * #982 Learned While Devving * #982 indicated 3 errors coming from atty. Here is how each error type was resolved 1. `ESM (dynamic import only)` - this is more of a warning than an error. It's warning, rightly, that versions of node that only use CommonJS requires (e.g. `require("@web3-storage/did-mailto")`) won't be able to load these libraries. i.e. these libraries are "ESM-only" in that they only work on runtimes that can do ESM-style `import()`. By default, attw warns on this, but this PR configures attw to ignore instead (via .attw.json https://github.com/web3-storage/w3up/pull/1004/files#diff-a3be4137698f4bdc5777d3831fac4e67d11c102c5e57dd54906ff333cb551aff ). 2. [internal resolution error](https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/InternalResolutionError.md) * Insight via https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/InternalResolutionError.md#declarations-and-javascript-match-but-are-not-checked-with-correct-settings > This problem can occur with tsc alone if the library author compiles with --moduleResolution node (also known as node10) or bundler, especially in combination with a package.json that has "type": "module" * This attw error was resolved by this PR but this PR changing all packages `tsconfig.json` to use moduleResolution=NodeNext (not `Node`). To do that, many type imports had to change so the imported module identifier includes a file path suffix 3. [no types](https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/UntypedResolution.md) - Most of these were due to the relevant package having sub packabe submodule listed in package.json `exports` but without a corresponding js file existing (e.g. it was an old package submodule that has since been removed) Also * added an npm script `attw` to run attw for the package. Now can run attw for all packages like `pnpm -r attw` from monorepo root * gitlab ci workflows for the two packages shown in #982 now run attw
After the fix from #1004, which affected how npm/pnpm are configured in package.jsons in a way that could affect the release process of our packages, I wanted to do a test release of a package. I just did it with did-mailto and the release went ok and I confirmed the new release of did-mailto resolves arethetypesworking errors as I expected #1013 (comment) I chose did-mailto because it's the simplest package and in case things went haywire, I think it has the least external dependers. I'm more worried about the other 3 packages (*-client) that I touched in this PR. https://github.com/web3-storage/w3up/pull/1012/files |
The original issue here shows arethetypesworking errors for the two packages of I've now released new versions of each and tested that arethetypesworking is happier about them now
|
Here are results of testing the other packages
|
…passes due to removal of chain-tracker export (#1014) Motivation: * #982 * I was testing the filecoin-client 2.0.1 release I did against attw and noticed it actually still fails for a tiny reason. https://arethetypeswrong.github.io/?p=%40web3-storage%2Ffilecoin-client%402.0.1 Change * now CI for filecoin-client will use attw to prevent releases like this that dont pass attw check * also update package.json to resolve the attw update for filecoin-client
@travis would you agree this issue is now resolved? |
Plugging a couple of our libraries into https://arethetypeswrong.github.io/ shows that our types could use some improvements:
https://arethetypeswrong.github.io/?p=%40web3-storage%2Fw3up-client%409.0.0
https://arethetypeswrong.github.io/?p=%40web3-storage%2Faccess%409.0.0
Fortunately they also have a wonderful key for the errors they show, with a helpful page for fixing each of them:
https://github.com/arethetypeswrong/arethetypeswrong.github.io#readme
We should fix the types on the most forward-facing stuff first because I think this is actively hampering downstream development with our libraries, and it would be good to check all of our libraries soon.
The text was updated successfully, but these errors were encountered: