For @mdx-js/react, move @types/react from dependencies to peerDependencies

[unrevised6419]

In the @mdx-js/reaact is mentioned this:

To enable types for imported .mdx, .md, etcetera files, you should make sure the TypeScript JSX namespace is typed. This is done by installing and using the types of your framework, such as @types/react.
– https://github.com/mdx-js/mdx/tree/main/packages/react#types

So it says that the developer itself should install the JSX types he needs, @types/react or maybe others like @vue/runtime-dom

But there is a problem, @types/react is included in package dependencies, and is creating conflicts with other JSX types, like @vue/runtime-dom, because TypeScript prefers as default @types/react, if it will find it in node_modules

From where this discussion was born?

When having a Vue/Nuxt project, it recommends using Volar extensions that will deal great with TypeScript and adds @vue/runtime-dom JSX types by default implicitly.

At the moment you want to integrate Storybook, the problem appears, because it will add @types/react as a transitive dependency from @mdx-js/react

npm ls @types/react
[email protected] /Users/iamandrewluca/Projects/the-project
└─┬ @storybook/[email protected]
  └─┬ @storybook/[email protected]
    └─┬ @mdx-js/[email protected]
      └── @types/[email protected]

Now TypeScript will see @types/react in node_modules and all your JSX in Vue will be Typed as React.

The current solution is to do some hacks to stub @types/react to an empty package

npm ls @types/react
[email protected] /Users/iamandrewluca/Projects/my-project
├─┬ @storybook/[email protected]
│ └─┬ @storybook/[email protected]
│   └─┬ @mdx-js/[email protected]
│     └── @types/[email protected]
└── @types/[email protected] -> ./stub/types__react

Now TypeScript will see that there is not JSX namespace in @types/react and will go back to using @vue/runtime-dom

The suggestion from our side would be to move @types/react from dependencies to devDependencies so it wont become a transitive dependency.

This was discussed in depth in some threads:

• JSX issues in template vuejs/language-tools#592
• Storybook v6 + vue + tsx -> tsc errors: conflict types with @types/react storybookjs/storybook#12505

[wooorm]

Hey!

@types/react is used in @mdx-js/react. It’s an actual dependency. It’s needed:

mdx/packages/react/lib/index.js

Line 2 in f4d78be

* @typedef {import('react').ReactNode} ReactNode

Without it, this project can’t be used in TypeScript.

[wooorm]

I see no problem that can be solved here, as expression a couple of times in the thread.

a) we’re talking about @mdx-js/react here, a react specific package
b) we’re talking about a direct dependency that is used in the code. Optional dependencies are things that a package manager will try to install, but if it fails, it’s silent. This is needed for os or platform specific packages. Native stuff. Not for types. There’s no optionalDependenciesForTool: {typescript: {'@types/react': '...'}} field. Peer dependencies are for things like react which are host packages. If X has a peer dependency on Y, the user will use those tools together. That’s React for example. That’s not what happens here either.
c) even if we went with peer dependencies or optional dependencies (https://github.com/orgs/mdx-js/discussions/2238#discussioncomment-4788689), it doesn’t solve this issue, because npm will install it anyway, and all package managers do things differently (https://github.com/orgs/mdx-js/discussions/2238#discussioncomment-4789185), and presumably lots of packages will use @types/react as a dep or peer dep too.
d) the issue is in TypeScript: https://github.com/orgs/mdx-js/discussions/2238#discussioncomment-4789185

(Aside, storybook probably doesn’t need this package. You can pass components to MDXContent fine. No provider needed. Only when you want a context based API, this is useful, which you probably don’t.)

[IanVS]

we’re talking about a direct dependency that is used in the code.

It is not used in the runtime code, only during development of the code and bundling, right? The types compile out of the runtime, for those using JavaScript and not TypeScript. This is why it would be added to devDependencies in addition to peerDependencies.

even if we went with peer dependencies or optional dependencies, it doesn’t solve this issue, because npm will install it anyway,

That used to be true, but is not the case in version 9 of npm.

presumably lots of packages will use @types/react as a dep or peer dep too.

I don't know of any other packages which use it as a direct dependency, though there might be some. IIRC testing-library had it as a dependency for a while, and it caused a ton of problems, so they moved it.

[wooorm]

@types/react is used in the types we ship.
It is correct that the types are not used by people who do not use TypeScript, but we do ship them, that’s a bummer, it adds some weight.
But we ship types, for users who use TypeScript, and those types we ship need the types they depend on too.
peerDependencies does not solve that.

That used to be true, but is not the case in version 9 of npm.

Do you have a reference? This is not mentioned in the changelog: https://github.blog/changelog/2022-10-24-npm-v9-0-0-released/.

[IanVS]

Do you have a reference?

No, it was unclear to me in the docs, so I tested it out in an experiment. I commented on this discussion as well, and someone chimed in that npm is not installing optional peerDependencies, which is also what I found in my test.

[wooorm]

Good to know.
It doesn’t sound great to me that npm made an undocumented change and that we should go all in on it.

I see that theoretically, moving frameworks to devDependencies, peerDependencies, and adding them to peerDependenciesMeta as optional: true, can be an improvement.
What makes me uneasy about this change is that our packages are operating at a humongous scale. Users will also use other package managers (yarn, pnpm), and will use outdated package managers. Some people will not have @types/react.
I think it’s more likely that we break peoples builds than that we improve them.

Then, this only accidentally fixes the problem at hand here (https://github.com/orgs/mdx-js/discussions/2238#discussioncomment-4788837, https://github.com/orgs/mdx-js/discussions/2238#discussioncomment-4789185).
A fix for that problem is external: it’s in TypeScript. Or in frameworks like React. Vue just landed this: vuejs/core#7958. React should do the same.