Add types

[TaylorBeeston]

Closes GH-13.

This PR adds the types directory that contains an index.d.ts file that allows TypeScript projects to use this package more easily. Additionally, there are some tests to ensure the correctness of this type definition, and some boilerplate associated with dtslint. package.json has been updated to include @types/hast and dtslint as dev dependencies. There is also the addition of the test-types script to run dslint, and index.d.ts is exported as the types file for this project.

This is my second time writing a .d.ts file for a project, so please forgive me if there are any issues. One thing I would like to note is that I'm not 100% how this will work with s instead of h. I'm fairly certain that it will get the same type definition, which should be flexible enough to work with both functions, but I have been having a hard time finding this information out for certain.

[ChristianMurphy]

LGTM, thanks @TaylorBeeston! 🙇

[wooorm]

@ChristianMurphy Any thoughts on the h vs. s thing from the PR comment?

[ChristianMurphy]

Any thoughts on the h vs. s thing from the PR comment?

Thanks for drawing my attention to that, I missed it the first read through.
That currently will not be supported, this maps to index.js, svg.js will not pick up on the typings.
TypeScript is a bit finicky when it comes to files that are not the entrypoint.
It wants the typings names the same, and next to the file.
For example:

index.js
index.d.ts
svg.js
svg.d.ts

We may need to refactor this to flatten the typings out to the top level to support this.

[TaylorBeeston]

We may need to refactor this to flatten the typings out to the top level to support this.

I wonder if we can get around this by changing the tsconfig file? I will try and experiment with this.

[TaylorBeeston]

I'm not sure if this is the best way to do it or not as I'm still a little new to working with Github, but I have pushed the commit Proposal to handle s that I think will handle our h vs s problem. I added another type declaration, svg.d.ts, and added that to the tsconfig. It looks like dtslint is correctly picking this up, and causing tests to pass. While I'm not 100% sure this fixes it, I did also update types in package.json to reference the entire types directory, and added svg.d.ts to files, so I believe this should work.

[wooorm]

I believe we’re putting dependent types in dependencies directly now, not in dev-deps.

[wooorm]

Suggested change

	"dtslint": "^3.6.12",
	"dtslint": "^3.0.0",

[ChristianMurphy]

I'm not sure the path trick in tsconfig is working, here's how to test.

inside hastscript

npm install
npm link .

outside hastscript create a new folder with

index.ts

import s = require("hastscript/svg")

s("path")

and tsconfig.json

{
  "compilerOptions": {
    "lib": ["es2015"],
    "strict": true,
    "baseUrl": "."
  }
}

in the new folder run

npm link hastscript

then test by running

npx -p typescript -p ts-node ts-node index.ts

(or running ts-node index.ts directly if you have it installed globally)

Currently I'm getting

Could not find a declaration file for module 'hastscript/svg'. '~/test/node_modules/hastscript/svg.js' implicitly has an 'any' type.
  Try `npm install @types/hastscript` if it exists or add a new declaration (.d.ts) file containing `declare module 'hastscript/svg';`

1 import s = require("hastscript/svg")
                     ~~~~~~~~~~~~~~~~

[TaylorBeeston]

@ChristianMurphy gotcha. Thanks for explaining how to test this! I hadn't thought to use npm link like that, and have mostly been testing out changes by installing this as a dependency and editing the files in node_modules. I've been doing some looking around and it looks like the only solutions are to ask people to add paths to their tsconfig's if they want to use s (yuck), flatten the typing to the top level like you've suggested above, wait for TypeScript to add support for multiple exports (microsoft/TypeScript#8305), or to use named exports and break backwards compatibility =/

[wooorm]

Flattening the types then seems the most reasonable?

[ChristianMurphy]

Flattening the types then seems the most reasonable?

Agreed, it's what we've done for other packages that have multiple files that need typings, like https://github.com/syntax-tree/unist-util-is

[TaylorBeeston]

I have just pushed out three commits, two of them are the suggestions by @wooorm above regarding package.json, and the third flattens out all the type definitions following the example of unist-util-is. I tested this out using the link trick, and everything made it through!

By the way, unist-util-is has @types/mdast listed as a dev dependency, just so you know

[ChristianMurphy]

By the way, unist-util-is has @types/mdast listed as a dev dependency, just so you know

That is intentional, it is only used in the tests
https://github.com/syntax-tree/unist-util-is/blob/79a313ffefc427fc08b1f6e9433d13095defed24/unist-util-is-test.ts#L2

[ChristianMurphy]

LGTM, thanks @TaylorBeeston! 🙇

[wooorm]

Sweet, thanks Taylor! Released!