Add type definitions

[Rokt33r]

This is the first try to implement type definitions for unified.

I knew there is another attempt to adding types. But, I don't think the processor should persist AST type. It would make users confused because it looks tricky and doesn't work well with presets.

https://github.com/hrajchert/unified-typings-test/blob/master/src/types/unified/unified.d.ts

[wooorm]

Nice work!! 💪

Some nits inline, and two big things:

• In all cases where a human can give a vfile, it could also be a Buffer or string. You added that in most places I think. But, maybe we should use VFileOptions there. As it supports some other stuff as well. Does it make sense to export VFileOptions from vfile, and use it here instead of this list of VFile | Buffer | string?
• Can we run prettier on the typescript files as well?

[Rokt33r]

While thinking about my typo, paris, it would be nice if we could have a documentation generator, like https://github.com/TypeStrong/typedoc, to generate document from .d.ts file. (Although we need more time to do.)

[wooorm]

Nice work, two more questions!

[wooorm]

Alright, so we should release this. As a minor I think? It’s not breaking in any way right?

[Rokt33r]

It won't break anything. Releasing as minor version should be fine. 👍

[ChristianMurphy]

Thanks @Rokt33r!

[wooorm]

Released! Thanks @Rokt33r, and also @hrajchert!

[rjimenezda]

This broke a project I have that uses both stylelint and Stencil 0.13. The dependency chain made me end up here.

So any projects that were using Typescript 2.x could be broken? I might have something wrong on the compiler settings, though. As I said, this gets installed on my package because of stylelint, so I don't know why tsc would parse these definitions 🤔

[wooorm]

Huh, weird, @Rokt33r, any thought on this?

[ChristianMurphy]

@rjimenezda

This broke a project I have

What is the error message/stacktrace?

I might have something wrong on the compiler settings, though.

What is the tsconfig for the project?

[ChristianMurphy]

similar to vfile/vfile-message#3

[ChristianMurphy]

Discussion on supported TypeScript versions started on Spectrum https://spectrum.chat/unified/type-definitions?thread=7c1f6266-6be2-4d0a-b599-4ff93a6d0efd

[rjimenezda]

@ChristianMurphy

It's complicated, because Stencil uses typescript under the hood, so I'm not 100% sure of the settings. If you're curious, it's here

Just to verify, I set up a quick project with typescript 2.9 and stylelint as dependencies with a simple tsconfig:

{
    "compilerOptions": {
        "module": "system",
        "noImplicitAny": true,
        "removeComments": true,
        "preserveConstEnums": true,
        "outFile": "out.js",
        "sourceMap": true
    },
    "include": [
        "src/**/*"
    ],
    "exclude": [
        "node_modules",
        "**/*.spec.ts"
    ]
}

If you run tsc with this config the following error a few times:

> tsc
node_modules/@types/unist/index.d.ts:31:20 - error TS2304: Cannot find name 'unknown'.

31     [key: string]: unknown;

On the sample project, the build actually finishes. Stencil seems to stop what it's doing if it encounters an error, though.

In any case, I was able to suppress this error by adding "skipLibCheck": true to the compiler options. This is kind of annoying (TS's fault) because I can't be selective about it.

It seems like tsc somehow automatically parses all definitions in node_modules/@types, because I have at least one other .d.ts files in node_modules using 'unknown' and it's being ignored.

I've actually checked the DefinitelyTyped repo and there's only a handful of projects that use unknown, so I wonder if this will gradually become more of common issue. I can't find anything regarding them having a way to control typescript versions.

The worst part about this is that I'm not even actually using this library, otherwise I'd be super happy to update and get the definition files 😂

[ChristianMurphy]

It's complicated, because Stencil uses typescript under the hood, so I'm not 100% sure of the settings.

This should be resolved by stenciljs/core@127f0d5 in stencil version 0.16.0 and above.

[rjimenezda]

Of course it will. We haven't upgraded yet, but we're planning on doing so eventually.

This will still happen to every single project that uses typescript < 3 and for whatever reason, this library is in their dependency graph.

In any case, I'll keep this in mind the next time I have to add types to a library through @types and make it a major, to minimize cases like mine.

[Rokt33r]

Sorry, it was a false alarm. It works fine with vscode editor but doesn't work with dtslint.

---

@wooorm It seems to be possible to move index.d.ts to the root dir of the project.

Like the below:

test/types/test.ts
test/types/tsconfig.json
test/types/tslint.json
index.d.ts

by updating tsconfig.json:

{
  "compilerOptions": {
    "lib": ["es2015"],
    "strict": true,
    "noImplicitAny": true,
    "noImplicitThis": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "baseUrl": ".",
    "paths": {
      "unified": ["../../index.d.ts"] // <-- here
    }
  }
}

If the file structure looks good, I'll fix this too after we resolve the versioning problem.

[Ore4444]

Yes, adding native typings can break projects that coded their own typings for Unified

For example, it broke my Unified service which had the following:

type UnifiedPluginFn = (opts: unknown) => (tree: any) => void
type UnifiedPlugin = UnifiedPluginFn | [UnifiedPluginFn, any]

interface IUnifiedProcessor {
    use: (plugin: UnifiedPlugin, opts?: unknown) => void
    process: (htmlInput: string) => any // vfile
}

Of course, IUnifiedProcessor is not compatible with unified.Processor.
nvm i fixed it.

[artberri]

Hello,

As it is said in #43 (comment) and #43 (comment) this upgrade will affect "every single project that uses typescript < 3 and for whatever reason, this library is in their dependency graph".

This happened to a project I'm working on because it depends on stylelint, and now we need to freeze the unified version because we don't want to add "skipLibCheck": true and we can't upgrade to Typescript 3 yet (I'm investigating if by using the types option in the compiler options I can be more selective than using skipLibCheck).

I see some discussions, like this microsoft/TypeScript#9731, about if @types should be a dependency, a dev dependency or a peer dependency in order to avoid these issues but I'm not sure if there is a "good" answer yet.

Are you thinking on doing something on this project (like bumb new mayor version, or remove the types as direct dependencies in 7.1.1,...)? Any suggestion on what should we do?

Thank you in advance.

[ChristianMurphy]

See the TypeScript definition discussion here https://spectrum.chat/?t=7c1f6266-6be2-4d0a-b599-4ff93a6d0efd