feat: ts declarations

[alanshaw]

This is maybe not quite right, but thought I'd PR what I had to get the ball rolling ;)

It would be a breaking change for consumers.

I think index.d.ts is ignored when publishing but this works for me locally using npm link.

[rvagg]

Why do you want it to be a default export?

tbh I don't have strong opinions on this question but we've established a convention so far of not exporting a default object, but as separate pieces you can pull out, import { encode } from '@ipld/dag-cbor'. If we switch here then we should switch the others to match but it'd be nice to have a strong rationale for it.

@mikeal and @gozala might have stronger opinions on this particular question.

Re distributing - you'll need to add a cp into the "build" script for it, like this: https://github.com/rvagg/cborg/blob/8c12677f9339b23a65af2f96fad40c6270cbf452/package.json#L12

[alanshaw]

Why do you want it to be a default export?

It is how it is in the codecs in the multiformats library e.g. https://github.com/multiformats/js-multiformats/blob/master/src/codecs/json.js so I changed it here for consistency and reuse of the Codec type from that library.

Re distributing - you'll need to add a cp into the "build" script for it

Ok, I'll add that.

[rvagg]

Oh interesting, so you're consuming Codec in TS but not in JS. There's been some disagreement about consuming Codec at the edges to date. Maybe this is a compromise solution? Or maybe it just confuses matters because it's a Codec in TS but not in JS now and it should probably be in both or not at all.

[mikeal]

ya, i was more convinced of the utility of individual exports 4 months ago than i am now. i use this codec a bunch of places now and just end up using ‘import * as codec’ everywhere.

[rvagg]

I think @ipld/car is the only place I've been very selective with imports because I only need one thing in each place: https://github.com/ipld/js-car/blob/16d11873a33ae1935beb17847de6fbb90faba1d3/lib/encoder.js#L4, but it wouldn't be a big deal to change.

[aschmahmann]

what's the status of this PR?

[rvagg]

@aschmahmann close! I'm trying to fix up some TS problems with js-multiformats which is the place where most of these issues focus on. Once we have a pattern there I was planning on updating all of these adjacent repos, including this one, and while doing that, just adopt this change and make it apply to dag-json and dag-pb as well. Will hopefully be done in the next day or two, this bundle of concerns is my current top code priority.

[rvagg]

OK, I've done a few things to this branch:

1. I've properly imported multiformats/codec/codec and wrapped the export in its codec(), so it now matches raw and json, e.g. https://github.com/multiformats/js-multiformats/blob/master/src/codecs/json.js - this doesn't do much but as long as we're doing it in TS we may as well go whole-hog on this type here.
2. Made it a breaking change because of the export default now.
3. Added basic typing information internally and implemented type checking as part of the build process (also needed to add a couple more typedefs into cborg in the process).
4. Removed the manual index.d.ts and made it auto-generate on build.

Does index.d.ts need any signalling? Will typescript look for this file without any other signalling, like "typesVersions" in package.json? This package isn't published with a corresponding index.js, it doesn't even have a "main". Or should we also add a "typesVersions" to make it pick up index.d.ts properly? (Then it could go into types/ along with the index.d.ts.map that's generated).

If this is all good, then I can get this published and do the same for @ipld/dag-json and @ipld/dag-pb.

@alanshaw @achingbrain ptal

[rvagg]

btw the auto-generated index.d.ts looks like this:

declare var _default: import("multiformats/codecs/codec").Codec<"dag-cbor", 113, any>;
export default _default;
//# sourceMappingURL=index.d.ts.map

compared to Alan's original:

import { codec } from 'multiformats'
declare var _default: codec.Codec<'dag-cbor', 113, any>
export default _default

[alanshaw]

Does index.d.ts need any signalling? Will typescript look for this file without any other signalling, like "typesVersions" in package.json?

Where does it end up? In my experience if it's next to the index.js it doesn't need anything.

[rvagg]

Where does it end up? In my experience if it's next to the index.js it doesn't need anything.

no, it's not, the distributable looks like this now:

./index.d.ts.map
./README.md
./package.json
./index.d.ts
./LICENSE
./esm
./esm/index.js
./esm/package.json
./esm/browser-test
./esm/browser-test/test-basics.js
./esm/node-test
./esm/node-test/test-basics.js
./cjs
./cjs/index.js
./cjs/browser-test
./cjs/browser-test/test-basics.js
./cjs/node-test
./cjs/node-test/test-basics.js

With "exports" doing this:

  "exports": {
    "browser": "./esm/index.js",
    "require": "./cjs/index.js",
    "import": "./esm/index.js"
  },

So maybe safer to use "typesVersions"?

[achingbrain]

As a rule of thumb, use typesVersions when you want to allow deep imports:

import { foo } from 'bar/src/foo'

Use types when you only want exports from the root:

import { foo } from 'bar'

Generally prefer exports from the root only as it means you can move files around in your module without breaking the API contract, and you can also do ./esm/.. ./cjs/.. magic again without breaking anyone.

If your index.d.ts is not in the root of the module (or next to where main points to, I guess?), add a types field to package.json pointing to it. You can use typings instead as in this PR, but we've settled on types everywhere else and consistency would be preferable.

We're putting a 'Best practices' doc together here: https://github.com/ipfs/aegir/blob/master/md/ts-jsdoc.md - it's got our learnings from the mammoth IPFS typing exercise and is worth reading.

[Gozala]

index.d.ts has no special meaning, TS just uses regular resolution rules but tries .d.ts file extension first. One caveat is TS does not (yet) supports exports field in package.json. That said as per @achingbrain I'd just use types field in package.json unless package provides multiple modules, in later case you'd need typeVersions.

[Gozala]

I have not realized @rvagg changed this pull to generate types and end up duplicating that effort in #15. I've merged my branch into this now. If it's no good feel free to revert or reset head back to where it was