Skip to content

Handle query parameters in Vite #2629

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

Merged
merged 2 commits into from
Aug 7, 2025
Merged

Conversation

markdalgleish
Copy link
Contributor

@markdalgleish markdalgleish commented Aug 7, 2025

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and discussions and couldn’t find anything or linked relevant results below
  • I made sure the docs are up to date
  • I included tests (or that’s not needed)

Description of changes

This fixes an issue where MDX imports with query parameters aren't picked up by the Rollup plugin in Vite. To fix this, this PR uses the extnamesToRegex util to create a regular expression for matching MDX extensions, and this util already handles the existence of query parameters correctly.

I ran into this in the context of React Router where I'm working on RSC support for Framework Mode which uses Vite. We're using query parameters to transform client and server versions of route files, but this approach means that MDX routes are no longer working since the query parameters cause the MDX plugin to skip over them.

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Aug 7, 2025
Copy link

codecov bot commented Aug 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (def2cba) to head (7b7ca45).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #2629   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines         2650      2650           
  Branches         2         2           
=========================================
  Hits          2650      2650           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@remcohaszing remcohaszing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I have one comment.

@@ -79,22 +82,20 @@ export function rollup(options) {
development: env.mode === 'development',
...rest
})
extnameRegex = extnamesToRegex(formatAwareProcessors.extnames)
},
async transform(value, path) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently this path variable is not really a path. Instead, it’s an ID (constructed from the path). It may contain query parameters.

I think the proper fix would be to rename this variable to id (including in the types). Then construct the path using:

const [path] = id.split('?')

The main difference is that remark/rehype/recma plugins also have access to the VFile. It’s important that it has the correct data.

@markdalgleish
Copy link
Contributor Author

@remcohaszing Thanks for the feedback. I've just pushed an update.

@wooorm wooorm merged commit 2b3381a into mdx-js:main Aug 7, 2025
6 checks passed

This comment has been minimized.

@wooorm wooorm added 👶 semver/patch This is a backwards-compatible fix 💪 phase/solved Post is done labels Aug 7, 2025
@github-actions github-actions bot removed the 🤞 phase/open Post is being triaged manually label Aug 7, 2025
@wooorm
Copy link
Member

wooorm commented Aug 7, 2025

Cool, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 phase/solved Post is done 👶 semver/patch This is a backwards-compatible fix
Development

Successfully merging this pull request may close these issues.

3 participants