Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
lensbart opened this issue Nov 23, 2024 · 9 comments
Assignees

Comments

@lensbart
Copy link

What version of React Router are you using?

7

Steps to Reproduce

  • Have .mdx file(s) in your routes folder
  • Follow the migration steps to react router 7
  • Run npm run typecheck
  • Observe type errors in the generated type files for the .mdx routes

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 lensbart added the bug label Nov 23, 2024
@lensbart
Copy link
Author

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

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

@lensbart
Copy link
Author

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
Copy link
Member

timdorr commented Nov 24, 2024

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
Copy link

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
Copy link
Author

lensbart commented Nov 24, 2024

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
Copy link
Member

timdorr commented Nov 24, 2024

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

@pcattori
Copy link
Contributor

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.

@GuillaumeQuenneville
Copy link

GuillaumeQuenneville commented Dec 1, 2024

I understand that it looked sketch in the file based routing but the layout mdxlayout.tsx and route post.tsx in the routes.ts was pretty intuitive.

But i don't think you can selectively apply layouts for mixed tsx and mdx routes at the same parent url with the file based routing so I can see a reason to do something else.

It would be nice to retain a solution to grab all the headings and titles from the content collection to dynamically make cards/links and table of contents. The way I got it working was pretty hacky with the rehype plugin and vite.glob so a better solution would be cool.

I'll be happy with any robust solution. Thanks for working on it!

@nowells
Copy link

nowells commented Dec 4, 2024

I ran into this same issue, and took a stab at fixing it in #12453

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants