remix-run / react-router

Declarative routing for React
https://reactrouter.com
MIT License
53.25k stars 10.32k forks source link

[Bug]: Type errors for generated type files that correspond to `.mdx` route files #12362

Open lensbart opened 2 days ago

lensbart commented 2 days ago

What version of React Router are you using?

7

Steps to Reproduce

Expected Behavior

No type errors

Actual Behavior

The generated type files contain a line like

type Module = typeof import("../this-is-an-mdx-file")

The type error is as follows:

Cannot find module '../this-is-an-mdx-file' or its corresponding type declarations.ts(2307)
lensbart commented 2 days ago

Appending .mdx to the imported file name fixes the issue:

type Module = typeof import("../this-is-an-mdx-file.mdx")
lensbart commented 2 days ago

Creating a .d.ts file for every .mdx file with the following contents also fixes the issue.

export {}

These files should be colocated next to the corresponding .mdx file.

timdorr commented 2 days ago

Note that this isn't a React Router issue.

Can you try two things?

  1. Add "types": ["mdx"] to your tsconfig.json. You must have @types/mdx installed.

  2. Add this to an mdx.d.ts file in your project root:

declare module '*.mdx' {
  let MDXComponent: (props: any) => JSX.Element;
  export default MDXComponent;
}
kennygoff commented 2 days ago

Having this same issue in a project using @mdx-js/rollup with mdx files as routes. I tried the fix you suggested @timdorr and still not working. Here's the attempted fix on my project for reference with typecheck failing in CI. Looks like declare module '*.mdx' doesn't fix it because the mdx file types aren't the problem, the imports in the generated routes seem to be the issue, although I may be misunderstanding what adding your suggesting is doing.

Since the import generated by the react-router types doesn't include the .mdx extension it might be an issue with how the types are generated. Even with MDX's docs they mention importing files as import Example from './example.mdx', I think that is needed. Here's a snippet from my generated types

// .react-router/types/app/routes/+types/_articles.beyond-wave-echo-cave.ts
type Module = typeof import("../_articles.beyond-wave-echo-cave"); // this needs an mdx extension

Creating a .d.ts file for every .mdx file with the following contents also fixes the issue.

export {}

These files should be colocated next to the corresponding .mdx file.

For now I am doing this since I don't have many mdx files this is manageable as a temporary solution for me.

lensbart commented 2 days ago

Note that this isn't a React Router issue.

It is: React Router generates files with invalid imports. The corresponding .mdx files previously worked fine as routes.

Can you try two things?

This doesn’t fix the issue. The issue is due to missing file extensions. The TypeScript compiler allows for some implicit file extensions (.ts, .tsx, .js, .d.ts, and a few others), but .mdx is not one of them, and this is not configurable.

timdorr commented 2 days ago

Ah, I didn't catch this was for typegen. I'll let others respond to that since it's not my thing.

pcattori commented 20 hours ago

There are two main ways to handle MDX:

  1. As content: MDX is treated as data that you can pass in via loaders and then render client-side
  2. As routes: MDX is treated as code that gets transformed into JS at build-time

It's always been conceptually cleaner to treat MDX as content, but in practice this usually meant implementing and maintaining your own (cached) unify/remark/rehype pipeline. Its tricky to set up correctly and there are a bunch of footguns around how components/assets referenced by those MDX files get treated.

So for a while, we explored MDX as routes (for example, here's a demo I built https://github.com/pcattori/remix-blog-mdx). The nice thing about this approach is that all bundling/transforms are done via Vite. So that means you can use an out-of-the-box solution like @mdx-js/rollup and avoid most of the complexity by letting Vite handle caching, asset/component resolution, etc. But this always felt like a hack since you needed to set up your routing in just the right way to get this working and it wasn't the intuitive blog.tsx (layout) and blog/:slug.tsx (post) routing.

By the way, allowing non-JS/TS files (like MDX) to be routes adds tons of complexity that requires us to run a "child compiler" under-the-hood, which has been causing all sorts of issues.

I'm actively working on a framework-agnostic way to model content like MDX similar to how Astro does it. Hoping to show this off soon.