Skip to content
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

check .d.ts files #849

Closed
achingbrain opened this issue Jul 1, 2021 · 6 comments
Closed

check .d.ts files #849

achingbrain opened this issue Jul 1, 2021 · 6 comments

Comments

@achingbrain
Copy link
Member

We don't appear to run any sort of checking on .d.ts files, this means the syntax across modules is inconsistent and worse, we can ship with syntax errors and other obvious-in-hindsight bugs.

Some suggestions:

  1. Enable linting for .d.ts files
  2. Test .d.ts files to catch things like:
import type { Derpstore } from 'interface-datastore'

(1) is quite straight forward but for (2) maybe we could include a tsd test command?

This was referenced Jul 1, 2021
@achingbrain
Copy link
Member Author

cc @hugomrdias @Gozala

@Gozala
Copy link
Contributor

Gozala commented Jul 1, 2021

I have no strong feelings about it, that said it’s not just our linter, typescript itself (or at least vscode) seems to not report lot of issues either. I often change file extension to .ts and then back.

Aegir also has to do some special handling of .d.ts files.

All this custom setup and divergence from default toolchains makes things harder for everyone. I wonder what were the reasons to switching from .ts files to .d.ts (which worked as expected with native toolchains) and whether those reasons still make sense and do a outweigh tradeoffs.

@achingbrain
Copy link
Member Author

This is basically a nightmare. The only way to check your .d.ts files is to turn off skipLibCheck, then you end up checking everything in node_modules too, no way to exclude them. Ref: microsoft/TypeScript#30511

I wonder what were the reasons to switching from .ts files to .d.ts

Not sure we ever switched - we wanted .d.ts generated from jsdoc only initially since it doesn't require people to know ts to contribute.

We then backtracked a little and added .d.ts when it became apparent that jsdoc has a bunch of downsides like duplication of types and it being hard to define certain structures.

The tooling for .d.ts seems quite lacking, I think because they're not really supposed to be hand written so maybe defining types in .ts is the way forward in order to ensure higher quality until the situation improves.

@Gozala
Copy link
Contributor

Gozala commented Aug 25, 2021

Not sure we ever switched - we wanted .d.ts generated from jsdoc only initially since it doesn't require people to know ts to contribute.

We definitely have switched because typedefs I've originally put in ipfs-core-types had .ts extensions, but somewhere down the line they've turned .d.ts.

@Gozala
Copy link
Contributor

Gozala commented Aug 25, 2021

Checked history and it seems like this is a change that did it ipfs/js-ipfs@a418a52#diff-ecc32b520be57da677d45d1b0c7d5f883328286d8de03159de95e1eb680d08df

achingbrain added a commit to ipfs/js-ipfs-unixfs that referenced this issue Aug 27, 2021
`.d.ts` files are not type checked as they are usually machine generated.

Switch to using `.ts` files instead so we get some compiler safety.

Refs: ipfs/aegir#849
achingbrain added a commit to ipfs/js-ipfs that referenced this issue Aug 27, 2021
`.d.ts` files are not type checked as they are usually machine generated.

Switch to using `.ts` files instead so we get some compiler safety.

Refs: ipfs/aegir#849
achingbrain added a commit to ipfs/js-ipfs-unixfs that referenced this issue Aug 27, 2021
`.d.ts` files are not type checked as they are usually machine generated.

Switch to using `.ts` files instead so we get some compiler safety.

Refs: ipfs/aegir#849
achingbrain added a commit to ipfs/js-ipfs that referenced this issue Aug 27, 2021
`.d.ts` files are not type checked as they are usually machine generated.

Switch to using `.ts` files instead so we get some compiler safety.

Refs: ipfs/aegir#849
@achingbrain
Copy link
Member Author

Closing as the resolution is to declare types in .ts files in order to get a more compile-time safety from the compiler.

SgtPooki pushed a commit to ipfs/js-kubo-rpc-client that referenced this issue Aug 18, 2022
`.d.ts` files are not type checked as they are usually machine generated.

Switch to using `.ts` files instead so we get some compiler safety.

Refs: ipfs/aegir#849
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

No branches or pull requests

2 participants