Skip to content

fix(dev/typegen): fix non-JS/TS files #12453

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

nowells
Copy link

@nowells nowells commented Dec 3, 2024

#12440 introduced a fix for tsconfig, however, it broke non-JS routes (like .mdx)

When we have a file ./app/routes/test/index.mdx which is a route we get the following error:

.react-router/types/app/routes/test/+types/index.ts:10:29 - error TS2307: Cannot find module '../index.js' or its corresponding type declarations.

10 type Module = typeof import("../index.js")
                               ~~~~~~~~~~~~~

Copy link

changeset-bot bot commented Dec 3, 2024

⚠️ No Changeset found

Latest commit: 7814811

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Dec 3, 2024

Hi @nowells,

Welcome, and thank you for contributing to React Router!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].

Thanks!

- The Remix team

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Dec 3, 2024

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@nowells
Copy link
Author

nowells commented Jan 8, 2025

I have worked around this with patch-package locally, however, I think this will bite many people who use mdx in their sites, which I think is rather common, so hoping we can land this before too many people take a swing at adopting v7 (which has been a very smooth upgrade from remix!)

@timdorr timdorr requested a review from pcattori January 8, 2025 16:00
@pcattori pcattori self-assigned this Jan 8, 2025
@nowells nowells closed this Mar 18, 2025
@nowells nowells reopened this Mar 18, 2025
@nowells nowells changed the title Support moduleResolution Node16 and NodeNext for non-JS files. Fix typescript generation for non-JS/TS files (like MDX) Mar 18, 2025
@MichaelDeBoey MichaelDeBoey changed the title Fix typescript generation for non-JS/TS files (like MDX) fix(dev/typegen): fix non-JS/TS files Mar 18, 2025
@timdorr
Copy link
Member

timdorr commented Mar 18, 2025

Fixed some conflicts for you. Any chance we could add a test? Not sure if it's possible without a lot of work, but I figured I'd ask.

@pcattori should probably approve this, I'd imagine.

@nowells
Copy link
Author

nowells commented Mar 24, 2025

Thanks @timdorr! Let me know if there is anything else you need from me!

@brophdawg11 brophdawg11 removed their request for review March 31, 2025 19:47
@hsab
Copy link

hsab commented May 27, 2025

hi folks! any plans to get this merged? are there any fixes in the meantime?

@nowells
Copy link
Author

nowells commented May 27, 2025

Wondering if we can get some 👀 on this from @pcattori ?

@timdorr
Copy link
Member

timdorr commented May 27, 2025

Looks like it needs some merge conflicts resolved first.

@nowells nowells force-pushed the nstrite/typescript-non-js branch 3 times, most recently from ea82b61 to cd63ba4 Compare May 27, 2025 15:11
@nowells
Copy link
Author

nowells commented May 27, 2025

@timdorr thanks, I updated the patch.

@timdorr
Copy link
Member

timdorr commented May 27, 2025

Looks like that nuked your CLA signature. Mind adding that back too?

…JS routes (like .mdx)

When we have a file `./app/routes/test/index.mdx` which is a route we get the following error:
```
.react-router/types/app/routes/test/+types/index.ts:10:29 - error TS2307: Cannot find module ../index.js or its corresponding type declarations.

10 type Module = typeof import("../index.js")
                               ~~~~~~~~~~~~~
```
@nowells nowells force-pushed the nstrite/typescript-non-js branch from cd63ba4 to 7814811 Compare May 27, 2025 16:21
@nowells
Copy link
Author

nowells commented May 27, 2025

@timdorr womp, sorry about that. fixed and pushed!

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

Successfully merging this pull request may close these issues.

5 participants