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.

Sign up for GitHub

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

Jump to bottom

Fast refresh does not work correctly with MDX #13574

Closed
michel-kraemer opened this issue May 30, 2020 · 11 comments · Fixed by #15851
Closed

Fast refresh does not work correctly with MDX #13574

michel-kraemer opened this issue May 30, 2020 · 11 comments · Fixed by #15851
Assignees
@Timer
Milestone
iteration 6

Comments

@michel-kraemer
Copy link
Contributor

Bug report

Describe the bug

Instead of just replacing the contents, fast refresh performs a full page reload if it is an .mdx file.

To Reproduce

  1. git clone https://github.com/michel-kraemer/test-nextjs-fast-reload-mdx.git
  2. cd test-nextjs-fast-reload-mdx
  3. npm i
  4. npm run dev
  5. Open http://localhost:3000 in your web browser
  6. You should see Hello World!
  7. Change the file pages/index.mdx
  8. The full page will be refreshed!!
  9. Open http://localhost:3000/test in your web browser
  10. Change the file pages/test.jsx
  11. The text will be updated without a full page reload

Expected behavior

Instead of reloading the full page, only the contents should be updated.

Screenshots

–

System information

  • OS: macOS
  • Browser: chrome
  • Version of Next.js: 9.4.4
  • Version of Node.js: 14.3.0

Additional context

  • This has worked correctly in Next.js 9.3.x with HMR.
  • The issue can be reproduced, no matter if the .mdx file imports/exports a Layout or not.
@vishalvijay
Copy link

I can confirm this.

I just verified with my project. The page doing full reload on content change in.mdx file.

@Timer Timer added this to the 9.4.5 milestone May 30, 2020
@Timer Timer self-assigned this May 30, 2020
@remorses
Copy link
Contributor

remorses commented Jun 5, 2020 •
edited
Loading

MDX is not the problem, I managed to get fast refresh working

Fast refresh gets disabled when the page file exports non react stuff (constants, objects, ) that gets consumed in other react files, so make sure you don’t export variables from MDX

Also make sure that your main App component is a named function (should not be export default () => { ... })

If you want to try out fast refresh with MDX files try out Dokz

@Timer Timer added the kind: bug label Jun 6, 2020
@michel-kraemer
Copy link
Contributor Author

@remorses Thanks. What you're saying makes sense. But in my case, I don't have any App component and my MDX file does not export variables.

@timneutkens timneutkens modified the milestones: 9.4.5, backlog Jun 16, 2020
@aulneau
Copy link

aulneau commented Jul 21, 2020

I can confirm this is happening with using the latest canary versions on multiple mdx based sites.

@Timer Timer modified the milestones: backlog, iteration 6, iteration 5 Jul 22, 2020
@Timer Timer added the point: 3 label Jul 26, 2020
@remorses
Copy link
Contributor

MDX fast refresh does not work for 9.5.0 but works for 9.4.0 in some cases, i can’t find the reason though

@aulneau
Copy link

aulneau commented Jul 28, 2020 •
edited
Loading

Correct, I was trying to do some debugging but came up with nothing. I thought at first perhaps it was from using additional rehype / remark plugins, but none of the base mdx examples work with fast refresh. @timneutkens @Timer, curious if this might be a priority anytime soon!?

Timer added a commit to Timer/next.js that referenced this issue Aug 4, 2020
@Timer
Enable Fast Refresh in Custom Loaders
b66207b 
Next.js plugins like `@next/mdx` inject additional webpack loaders to compile files, but they omit the necessary loader for Fast Refresh to work.

Instead of making these files deopt out of Fast Refresh, we can automatically detect and inject the loader in these cases.

Fixes vercel#13574
@Timer Timer mentioned this issue Aug 4, 2020
Merged
Timer added a commit to Timer/next.js that referenced this issue Aug 4, 2020
@Timer
Enable Fast Refresh in Custom Loaders
9dec8c9 
Next.js plugins like `@next/mdx` inject additional webpack loaders to compile files, but they omit the necessary loader for Fast Refresh to work.

Instead of making these files deopt out of Fast Refresh, we can automatically detect and inject the loader in these cases.

Fixes vercel#13574
@kodiakhq kodiakhq bot closed this as completed in #15851 Aug 4, 2020
kodiakhq bot pushed a commit that referenced this issue Aug 4, 2020
@Timer
Run Fast Refresh Loader in Babel Loader (#15851)
754ec66 
Next.js plugins like `@next/mdx` inject additional webpack loaders to compile files, but they omit the necessary loader for Fast Refresh to work.

Instead of making these files deopt out of Fast Refresh, we can automatically detect and inject the loader in these cases.

Fixes #13574
@Timer
Copy link
Member

Timer commented Aug 5, 2020

Fixed in next@^9.5.2-canary.6 or newer.

@michel-kraemer
Copy link
Contributor Author

I can confirm it works with the latest canary. Thanks for the effort!!

michel-kraemer added a commit to steep-wms/steep-wms.github.io that referenced this issue Aug 5, 2020
@michel-kraemer
Update Next.js to 9.5.2-canary.6 to solve fast refresh issues
53103d9 
See vercel/next.js#13574
@weyert weyert mentioned this issue Feb 1, 2021
Closed
@gaearon
Copy link
Contributor

gaearon commented Feb 24, 2021

Just wanted to add I ran into this and then realized I had export const stuff = ... at the top of my MDX, which caused the bailout. I removed that and it worked.

@remorses
Copy link
Contributor

@gaearon is there a way to name non react component exports in a way that does not bail out react refresh? Maybe the babel plugin could ignore exports prefixed with __?

@karlhorky karlhorky mentioned this issue Aug 30, 2021
Open
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Timer Timer

Labels
None yet
Projects
None yet
Milestone
iteration 6
Development

Successfully merging a pull request may close this issue.

Run Fast Refresh Loader in Babel Loader Timer/next.js
8 participants
@michel-kraemer @Timer @gaearon @vishalvijay @timneutkens @aulneau @balazsorban44 @remorses