@types packages should be in devDependencies

[damonmaria]

Subject of the issue

This package includes @types/unist and @types/vfile as dependencies

Your environment

• MacOS 10.14.3
• node v10.15.2, yarn 1.13.0
• Package react-scripts@2.1.6 (from Create React App) includes unified

Steps to reproduce

1. yarn add unified
2. yarn why @types/node

Expected behaviour

No @types packages should be installed from adding unified.

Actual behaviour

Adding unified adds @types/unist, @types/vfile, @types/node and other packages into node_modules.

[ChristianMurphy]

/cc @Rokt33r

[Rokt33r]

@damonmaria @ChristianMurphy We've discussed this issue in here.

https://spectrum.chat/unified/type-definitions/where-should-we-put-types-unist~a22fb3db-88bf-472e-8cbb-9e4191116d6d

I also agree with @damonmaria 's opinion.

So I think the best solution is:

• Move @types/unist to devDependencies (without adding it peerDependencies)
• Add description, which is installing @types/unist, to readme.md or other place for typescript users.

[damonmaria]

The actual base issue I'm having is that @types/node, @types/react-native and @types/styled-components in some combination in different versions declare 'base things' (like console and FormData) differently. Which causes Typescript to bork. I'm in a web app so @types/node shouldn't even be there. But at the moment react-scripts is forcing that to happen (through this package).

Sorry, I can't speak to what unified should do as I'm not even a direct user of it.

[Rokt33r]

unified is a pluggable interface. It accept a parser, a compiler and some transformers. For example, remark is actually a unified instance with remark-parse and remark-compiler.

I think dedupe might help. https://yarnpkg.com/lang/en/docs/cli/dedupe/

But, it seems you need to install @types/node when using cra with typescript. https://facebook.github.io/create-react-app/docs/adding-typescript

[damonmaria]

Yes, I've seen the CRA recommendation for installing @types/node but it seems crazy since in a CRA app you don't have all of Node available. With @types/node hanging about you can do things like import { readFile } from 'fs' which in a web app, well... obviously isn't going to work.

I always switch @types/node out for @types/webpack-env which declares process.env and the webpack internals. process.env is all I think from @types/node which is actually available in a CRA/Webpack app.

[Rokt33r]

That's NOT crazy. React is designed to be isomorphic for ssr. Using @types/node is common when you are developing with webpack.

And unified is also needing it because it accepts Buffer.

[wooorm]

So I think the best solution is:

Move @types/unist to devDependencies (without adding it peerDependencies)
Add description, which is installing @types/unist, to readme.md or other place for typescript users.
— @Rokt33r

Could you elaborate on how that would affect users using unified deep down in, say, CRA?

[ybiquitous]

Hi, I agree with the idea to move @types/* packages to devDependencies, and I hope strongly. 🙏
Because remark doesn't work on this PR TypeStrong/ts-loader#908. 😭
(Please see also TypeStrong/ts-loader#908 (comment))

[wooorm]

@Rokt33r Do you have any thoughts on #45 (comment)?

[ybiquitous]

@wooorm Thanks! I am looking forward to the next release! 😄

[wooorm]

Coming soon to a package registry near you!

[osdiab]

Curious why there needs to be @types/vfile since it seems the vfile repo has types in it already? Are those types copied to DefinitelyTyped or something?

I ask since the inclusion of the types in DefinitelyTyped causes this library to depend on @types/node, which is causing my frontend app to have @types/node present. It happens because react-scripts many layers down depends on unified (react-scripts -> @svgr/webpack -> @svgr/plugin-jsx -> unified -> @types/vfile). And this is annoying since now the TypeScript compiler is telling me that I can use Node builtins and libraries like fs in my frontend web app, which in reality would make it crash at runtime.

[ChristianMurphy]

cross posting this: syntax-tree/unist-util-is#14
Yarn 2 may cause challenges with this approach.

[remcohaszing]

Continueing the discussion from rehypejs/rehype-document#9 (review):

I believe the only package causing issues is vfile, because it depends on Buffer.

In postcss it’s solved by not using string | Buffer, but string | { toString(): string }

https://github.com/postcss/postcss/blob/master/lib/postcss.d.ts#L24-L26

This allows to somewhat loosely depend on Buffer as a type, but not depend on the entire node types that cause conflicts in a dom or webworker environment.

[ChristianMurphy]

@osdiab you may be running an older version of Unified, unified doesn't depend on @types/vfile in the latest version, if you have follow up questions, feel free to open a thread at https://spectrum.chat/unified/type-definitions

[ChristianMurphy]

@remcohaszing Thanks for the idea, that approach is definitely worth exploring, opened vfile/vfile#54 with a version of this idea.

[osdiab]

@ChristianMurphy that is likely to be the case as my comment was from a year ago 😂 i don't think we're experiencing this issue now anymore but I'll double check it.