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

Fast refresh not working properly due to page reload when compiling middleware #31827

Closed
timfuhrmann opened this issue Nov 26, 2021 · 50 comments · Fixed by #33873
Closed

Fast refresh not working properly due to page reload when compiling middleware #31827

timfuhrmann opened this issue Nov 26, 2021 · 50 comments · Fixed by #33873
Labels
Webpack Related to Webpack with Next.js.

Comments

@timfuhrmann
Copy link
Contributor

timfuhrmann commented Nov 26, 2021

What version of Next.js are you using?

12.0.4

What version of Node.js are you using?

16.13.0

What browser are you using?

Chrome, Safari, Edge

What operating system are you using?

macOS

How are you deploying your application?

next dev

Describe the Bug

Since I don't know the technological background or necessity of the page reload for middleware, I'm not sure if that's something that you have to live with when working with middleware - but any change, like edits made to react components, often trigger a page reload due to the middleware being compiled:

wait - compiling /_middleware (middleware only)...

This becomes especially frustrating when working with getStaticProps.

Expected Behavior

No page reload when applying changes ("Fast Refresh").

To Reproduce

Basic middleware implementation.

@timfuhrmann timfuhrmann added the bug Issue was opened via the bug report template. label Nov 26, 2021
@timneutkens timneutkens added type: needs investigation Webpack Related to Webpack with Next.js. labels Nov 26, 2021
@exneval
Copy link

exneval commented Nov 28, 2021

I was able to reproduce this, Developer Experience when using middleware is bad, the fast refresh won't work properly (editing a simple changes will get a full reload), hope this can get further investigation

@dlindenkreuz
Copy link
Contributor

Possibly related: #30791

@iksent
Copy link

iksent commented Dec 6, 2021

I thought #31548 would fix this issue, but I am experiencing the same at the latest v.12.0.7

@havran

This comment has been minimized.

@balazsorban44
Copy link
Member

This has been fixed, please upgrade npm i next@latest.

@iksent
Copy link

iksent commented Dec 15, 2021

@balazsorban44, which version resolves this issue? next@latest is v12.0.7, and it wasn't fixed there.

@balazsorban44
Copy link
Member

🤔 I just tried it locally. Maybe I'm missing something?

@balazsorban44
Copy link
Member

Sorry. Could you try canary?

@balazsorban44
Copy link
Member

If anyone else still having issues, please open a separate bug report with a reproduction.

@timfuhrmann
Copy link
Contributor Author

Sorry, just tried canary again and it doesn't seem to be fixed, there still is a reload which I didn't realize because I worked on a project without getStaticProps.

@timfuhrmann
Copy link
Contributor Author

To reproduce, add some JSX and save your project, repeat 4 - 5 times and you should get a reload.

@balazsorban44
Copy link
Member

Yeah, still no problem on my side. 👀

Could you link to your repository? Create a minimal reproduction?

@balazsorban44
Copy link
Member

I don't have a TMDB API key, but for the sake of pinning down where the problem might be, it is favorable if you could provide an isolated/minimal reproduction anyway.

@balazsorban44 balazsorban44 reopened this Dec 15, 2021
@henriqemalheiros
Copy link

Hey! I was facing the same issue until I found the problem: something that is imported in the middleware and somewhere in the app. The import could be anything, including an external dependency. Here is the minimal reproduction: henriqemalheiros/middleware-fast-refresh.

To reproduce it, though, you need to wait ~20 seconds after the page is fully loaded or the last change was made. If you make a change before the ~20 seconds interval, the fast refresh works as expected. After that, any change to any part of the app triggers a full refresh.

There is a warning logged to the console, but it's to fast to actually read it. In next@canary, the same warning is shown by the app before the full refresh so we can read it:

About to perform a full refresh

Fast Refresh will perform a full reload when you edit a file that is imported by modules outside of the React rendering tree. It is also possible the parent component of the component you edited is a class component, which disables Fast Refresh. Fast Refresh requires at least one parent function component in your React tree.

You can find more information in the related Webpack error below:

Cannot read properties of undefined (reading 'call')

You can read more about Fast Refresh in our documentation.

@switz
Copy link

switz commented Dec 27, 2021

@henriqemalheiros nice work! this has been really hurting my productivity, thanks for coming up with a reproduction.

@lao9s
Copy link

lao9s commented Jan 8, 2022

Hi,
I have the same issue. To reproduce it: import a function in react component then the same function import in your _middleware.js. Then, if you have running dev server, stop it and start it: npm run dev, whait ~15 secconds then try to do changes on your component, will perform full reload.

As a temporary solution is to have different functions with the same logic and then import them separately into both files(your react component and _middleware.js)

Does anyone know how to solve this to use the same function in both places?

@switz
Copy link

switz commented Jan 8, 2022

This is pretty frustrating and severely hindering the development experience. Every time I update code it does a full refresh to a blank page and I have to refresh twice to get it to render.

I have imports in my _middleware – there's not really much I can do to get around that. Can I disable re-compilation on a particular nextjs path? That file isn't changing at all, it should be compiled once until any changes are made.

@tomduncalf
Copy link

tomduncalf commented Jan 11, 2022

Also seeing frequent reloads with 12.0.7 when working due to the middleware recompiling.

With canary it is perhaps a little less frequent, but instead of just a reload I first get this dialog - to be honest, it was better before, as I didn't have to click a button to reload also.

image

This is just doing/undoing the same change to a styled component over and over which works like 3 in 4 times, and does a full reload 1 in 4. I see wait - compiling /_middleware... in the console despite not changing the middleware.

My middleware is just the basic auth example and is totally self contained, except for importing TS types: import { NextRequest, NextResponse } from "next/server"; so it's not related to the shared imports some others have mentioned.

I can confirm that removing middleware.ts fixes this, but doing an early return from the middleware in dev mode does not help.

My project is too large and complex to share as a repro, but if no one else has created one I can try and create one.

@switz
Copy link

switz commented Jan 12, 2022

After upgrading to canary I see the dialog as well, but with a different error message. I didn't see anything related to on-demand-entries before upgrading to canary. Hoping this is helpful:

image

@WoutGroenendijk
Copy link

WoutGroenendijk commented Jan 13, 2022

I ran into the same issue after upgrading to next 12.0.8, this was caused by my custom imports in the _middleware. After removing the imports and moving them inside the _middleware file it all worked fine.

@tomduncalf
Copy link

@WoutGroenendijk you must still have import { NextRequest, NextResponse } from "next/server"; though, right? For me this happens just with that as the only import

@WoutGroenendijk
Copy link

@tomduncalf Yes I still got import { NextRequest, NextResponse } from "next/server"; I just moved my custom imports of path's and constants inside my middleware file, after that it started to work fine.

@henriqemalheiros
Copy link

@WoutGroenendijk you must still have import { NextRequest, NextResponse } from "next/server"; though, right? For me this happens just with that as the only import

@tomduncalf is there any chance you are importing NextRequest or NextResponse in other parts of the app, like an API route? I didn't check for imports in places other than React components and I don't use TypeScript, but it makes sense for the problem to occur in your use case as well.


@tomduncalf Yes I still got import { NextRequest, NextResponse } from "next/server"; I just moved my custom imports of path's and constants inside my middleware file, after that it started to work fine.

@WoutGroenendijk that's what I did as well:

import { API_URL } from '../lib/constants'

became

const API_URL = process.env.NEXT_PUBLIC_API_URL ?? ''

It solved the problem, but that makes the app harder to maintain and it is a change impossible to be made when importing other things, like a dependency (lodash, for example).

@tomduncalf
Copy link

Good question @henriqemalheiros, I’m not sure why I’d be importing them elsewhere but it’s possible, I’ll check. Hopefully this issue can be resolved soon anyway, feels like going back to the dark ages with full page refresh 😂

@switz
Copy link

switz commented Jan 27, 2022

Still seeing this issue on 12.0.9

image

@bmichotte
Copy link

image

Same here

@klapec
Copy link

klapec commented Jan 27, 2022

Yup, definitely not fixed on 12.0.9.
Here's the stacktrace that I'm getting:

TypeError: Cannot read properties of undefined (reading 'call')
    at Object.options.factory (http://localhost:3000/_next/static/chunks/webpack.js?ts=1643268216530:685:31)
    at __webpack_require__ (http://localhost:3000/_next/static/chunks/webpack.js?ts=1643268216530:37:33)
    at fn (http://localhost:3000/_next/static/chunks/webpack.js?ts=1643268216530:354:21)
    at eval (webpack-internal:///./app.config.ts:30:68)

As soon as I remove any module imports - the problem is gone.

@simontaisne
Copy link

Same here on 12.0.9 and without any import outside of next/server :(

TypeError: Cannot read properties of undefined (reading 'call')
    at Object.options.factory (http://localhost:3000/_next/static/chunks/webpack.js?ts=1643273407380:690:31)
    at __webpack_require__ (http://localhost:3000/_next/static/chunks/webpack.js?ts=1643273407380:37:33)
    at fn (http://localhost:3000/_next/static/chunks/webpack.js?ts=1643273407380:359:21)
    at eval (webpack-internal:///./src/hooks/useApollo.ts:15:40)

@bmichotte
Copy link

Hello @balazsorban44, I just created a quick reproduction repository : https://github.com/bmichotte/next-issue-31827

With that project cloned, you have to

  1. yarn install
  2. yarn dev
  3. open http://localhost:3000/ (and enjoy an Hello world :P)
  4. wait a few seconds, then uncomment the console.log in the api/v1/hello.ts file
  5. save that file
  6. reproduce 4-5 with commenting/uncommenting the console.log, you should see the full reload

@balazsorban44 balazsorban44 reopened this Jan 27, 2022
@iksent
Copy link

iksent commented Jan 31, 2022

Hi, this has been fixed on latest (as of writing 12.0.9) please upgrade and give it a try!

Just tried and now we need to close this annoying notice "About to perform a pull refresh" in addition to page refreshing 😆

@seonghyeonkimm
Copy link

seonghyeonkimm commented Jan 31, 2022

next: 12.0.9
node: 16.13.0

Everytime I edit any files in my project, I also run into this warning dialog. And I think it is related to @sentry/nextjs withSentryConfig call in next.config.js.
스크린샷 2022-02-01 오전 1 10 02

After removing withSentryConfig, I get the same warning dialog with different message.
스크린샷 2022-02-01 오전 1 06 41

@jescalan
Copy link
Contributor

jescalan commented Feb 1, 2022

Hi friends! Just want to say that we are aware of this issue on the next.js core team and are working towards getting a fix out. Really appreciate your patience here, and so sorry to everyone who this has inconvenienced!

@kodiakhq kodiakhq bot closed this as completed in #33873 Feb 1, 2022
kodiakhq bot pushed a commit that referenced this issue Feb 1, 2022
…_module (#33873)

fixes #31827



## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
@ijjk
Copy link
Member

ijjk commented Feb 1, 2022

Hi, this has been updated in v12.0.11-canary.1 of Next.js, please update and give it a try!

@bmichotte
Copy link

@ijjk I can confirm that I don't see the complete refresh in the two projects I tested with that version !

@switz
Copy link

switz commented Feb 2, 2022

Can also confirm this is fixed. Thank you so so so much!!

@GandharvJaggi
Copy link

GandharvJaggi commented Feb 3, 2022

HI, we are using a custom server.js . Upgrading to v12.0.11-canary.1 did not fix the problem.

EDIT: we are not using any kind of middleware.

@ijjk
Copy link
Member

ijjk commented Feb 3, 2022

@GandharvJaggi if you are not using middleware then this sounds unrelated to your issue, can you open a separate issue with a reproduction to allow investigation?

@pqt
Copy link
Contributor

pqt commented Feb 5, 2022

@jescalan & @ijjk since serving the public is an often thankless task, I just want to say thank you so much for this, it has really been a thorn in the side and this is probably the highlight to the day.

Just wanted to share that, pass it along to anyone involved, pats on the back for all.

oBusk added a commit to oBusk/npm-diff.app that referenced this issue Feb 6, 2022
@sebastienbarre
Copy link

sebastienbarre commented Feb 9, 2022

I tried v12.0.11-canary.10 and I still get a lot of those but in the context of using MDX files. Would you mind considering an option to disable this big alert box, and reload automatically? Or is there an easy to do it already?

Error: Aborted because ./pages/posts/my-mdx-page.mdx is not accepted
Update propagation: ./pages/posts/my-mdx-page.mdx -> ./node_modules/next/dist/build/webpack/loaders/next-client-pages-loader.js?page=%2Fposts%2Fmy-mdx-page&absolutePagePath=...posts%2Fmy-mdx-page.mdx!
    at applyHandler (http://localhost:3000/_next/static/chunks/webpack.js?ts=1644440995401:852:31)
    at http://localhost:3000/_next/static/chunks/webpack.js?ts=1644440995401:536:21
    at Array.map (<anonymous>)

When starting in dev mode, I noticed this message relate to fast refresh and a "custom loader", though I'm not sure what it is:

ready - started server on 0.0.0.0:3000, url: http://localhost:3000
info  - automatically enabled Fast Refresh for 1 custom loader

Thank you

@ijjk
Copy link
Member

ijjk commented Feb 10, 2022

@sebastienbarre could you open a fresh issue with a reproduction so we can investigate this case? It seems different from the original issue here.

@sebastienbarre
Copy link

Thanks for your time @ijjk. I created #34212, with a video capture and a repo.

kodiakhq bot pushed a commit that referenced this issue Feb 14, 2022
x-ref: #31827
x-ref: #34212

![image](https://user-images.githubusercontent.com/1365881/151994766-b9afb349-1a9a-4220-9387-de10165e34e3.png)




## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`


Co-authored-by: JJ Kasper <[email protected]>
natew pushed a commit to natew/next.js that referenced this issue Feb 16, 2022
…_module (vercel#33873)

fixes vercel#31827



## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Webpack Related to Webpack with Next.js.
Projects
None yet
Development

Successfully merging a pull request may close this issue.